docs: add AWS deployment guide + minor S3 test fixture update #1052
Conversation
…ation This document provides a comprehensive guide on deploying the Antenna backend to AWS, detailing the architecture, environment variables, and deployment workflow.
Updated the README to enhance clarity and detail about the Antenna backend deployment process, including infrastructure components and future improvements.
✅ Deploy Preview for antenna-preview canceled.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds AWS infrastructure documentation, Elastic Beanstalk deployment templates, an empty IAM policy placeholder, and a new dual-mode S3 test/demo storage module with helpers to create and populate S3/MinIO buckets. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
aws-infra/README.md
Outdated
| ### Step 1 — Build and push image to ECR | ||
|
|
||
| docker build -t antenna-backend . | ||
| docker tag antenna-backend:latest <ECR_URI>:v10 |
There was a problem hiding this comment.
is v10 an example version number? If so maybe add a small note about how to think about and use container versions
There was a problem hiding this comment.
Yes, v10 is just an example. I’ll add a note explaining how contributors should choose version numbers so it's clear and consistent.
|
|
||
| - `/api/v2/` returns `200` | ||
| - Django container remains healthy | ||
| - Celery worker connects to Redis successfully |
There was a problem hiding this comment.
Added a section that explains exactly how each validation step was performed (API 200 check, EB health check, Worker/Beat logs, and Flower UI access). This clarifies how each item was confirmed during deployment.
aws-infra/README.md
Outdated
|
|
||
| - /.ebextensions/00_setup.config : EB environment variables and settings | ||
| - /.ebignore : Exclusion list for EB deployment bundle | ||
| - /Dockerrun.aws.json : Multi-container EB deployment config |
There was a problem hiding this comment.
can we add example versions of these files?
There was a problem hiding this comment.
Absolutely! Some of these files may contain secrets, so I’ll sanitize the sensitive parts and upload minimal example versions.
…guration notes Added guidelines for Docker image versioning and deployment validation steps.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
aws-infra/README.md (5)
41-43: Minor: Improve sentence structure for formal documentation tone.The text uses an implied subject in the second sentence. Consider making the subject explicit:
- Environment type: - - Single-instance environment (used for development/testing to reduce cost). - - Can be upgraded later to a load-balanced environment for production. + Environment type: + - Single-instance environment (used for development/testing to reduce cost). + - This can be upgraded later to a load-balanced environment for production.
55-58: Optional: Tighten the explanatory note for conciseness.The phrase "originally created for" is slightly verbose. Consider simplifying:
- - Additional outbound egress security group - *(originally created for App Runner, now reused for EB networking)* + - Additional outbound egress security group + *(created for App Runner, reused for EB networking)*
334-341: Fix: Add missing language specifiers to code blocks and tighten adverb usage.Several code blocks are missing language identifiers, and one sentence has redundant adverb usage:
- Line 339: Remove repeated adverb for conciseness:
- Celery worker connects to Redis successfully - Celery Beat schedules run successfully + Celery worker connects to Redis + Celery Beat schedules run successfully
- Lines 351, 362, 396, 411, 425: Add language specifiers to fenced code blocks. For example:
At line 351, change:
- ``` + ```bashApply the same fix to lines 362 (
bash), 396 (bash), 411 (bash), and 425 (bashorhtml).
- Line 343: Remove blank line inside blockquote before "How to validate..." explanation for proper markdown formatting.
248-260: Fix: Reduce repetitive adjective usage in least-privilege recommendations.Lines 254-255 use the word "only" twice in consecutive bullet points. Vary the language:
- Restricting S3 access to only specific buckets - Restricting ECR access to only required repositories - Minimizing CloudWatch permissions + Restricting S3 access to specific buckets + Restricting ECR access to required repositories + Minimizing CloudWatch permissions
522-524: Fix: Use asterisks for emphasis instead of underscores (markdown convention).Line 524 uses underscores for italics, which is inconsistent with markdown best practices. Change:
-_End of documentation._ +*End of documentation.*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
aws-infra/README.md(1 hunks)aws-infra/policies/iam_elastic_beanstalk_policy.json(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
aws-infra/policies/iam_elastic_beanstalk_policy.json
[error] 1-1: Expected an array, an object, or a literal but instead found the end of the file.
Expected an array, an object, or a literal here.
(parse)
🪛 LanguageTool
aws-infra/README.md
[style] ~43-~43: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
[style] ~58-~58: This phrase is redundant. Consider writing “created”.
Context: ... outbound egress security group *(originally created for App Runner, now reused for EB netwo...
(ORIGINALLY_CREATED)
[style] ~255-~255: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...c buckets - Restricting ECR access to only required repositories - Minimizing Cl...
(ADVERB_REPETITION_PREMIUM)
[style] ~339-~339: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...uccessfully - Celery Beat schedules run successfully - Flower UI loads on port 5555 (if secu...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
aws-infra/README.md
343-343: Blank line inside blockquote
(MD028, no-blanks-blockquote)
351-351: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
362-362: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
396-396: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
411-411: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
425-425: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
524-524: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
524-524: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
🔇 Additional comments (3)
aws-infra/README.md (3)
307-321: Approve: Comprehensive responses to past review feedback.The developer has thoroughly addressed all prior review comments:
- ✓ Added version strategy guidance (lines 307–321) in response to version numbering questions
- ✓ Included detailed validation methodology (lines 344–435) explaining how each deployment step is confirmed
- ✓ Clarified Redis TLS fix location (lines 450–471) explaining the role of
.ebextensions/00_setup.configThese additions significantly improve clarity for maintainers and contributors.
Also applies to: 344-435, 450-471
1-524: Overall Assessment: Comprehensive documentation with strong team feedback integration.This README provides excellent coverage of the AWS Elastic Beanstalk deployment architecture, infrastructure components, deployment workflow, and troubleshooting guidance. The structure progressively builds from high-level overview → detailed configuration → practical deployment steps → validation methodology → common issues.
Strengths:
- Clear, logical section flow suitable for onboarding maintainers and contributors
- Detailed validation section with concrete examples (
curl,eb logs) helps teams confirm successful deployments- IAM and networking sections properly document current state and acknowledge least-privilege future improvements
- All four prior review comments from the team have been thoughtfully incorporated
Resolution needed before merge:
- Resolve the port inconsistency identified in the earlier comment (5000 vs 8000).
Optional polish items:
- Apply the markdown and style fixes noted in the separate comments above for professional consistency.
87-96: Verify port inconsistency claim: Django container port mapping (lines 91 vs. 376).The review identifies a discrepancy stating line 91 mentions Django listening on port 5000 (exposed as 80), while line 376 states port 8000. Unable to access the repository to independently verify these specific line references and claims. Please confirm:
- Actual port number configured in the Django container
- Whether both references in the document match the actual configuration
- Update the documentation to reflect the correct port mapping if inconsistency exists
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
File is empty and contains no valid JSON policy content.
The IAM policy file is currently empty and fails JSON validation. A valid Elastic Beanstalk IAM policy should follow the structure with "Version" and "Statement" keys, but this file contains none.
Since the PR adds comprehensive AWS infrastructure documentation, this policy file is critical infrastructure-as-code. An empty file cannot be used for actual deployment and contradicts the PR's intent.
Do you want me to generate a starter IAM policy tailored to the Antenna Elastic Beanstalk deployment? Based on the PR objectives mentioning an ECS-based multi-container setup with RDS and ElastiCache, I can provide a policy that aligns with those resources. Please clarify:
- Intended scope: Is this policy for the EC2 instance role (what EB instances assume) or for deployment/CI-CD users?
- Least-privilege requirement: Should the policy follow least-privilege with scoped resources, or use broader wildcards for initial setup?
- Additional integrations: Does it need permissions for ECR, RDS, ElastiCache, CloudWatch, or other services mentioned in the infrastructure guide?
🧰 Tools
🪛 Biome (2.1.2)
[error] 1-1: Expected an array, an object, or a literal but instead found the end of the file.
Expected an array, an object, or a literal here.
(parse)
🤖 Prompt for AI Agents
In aws-infra/policies/iam_elastic_beanstalk_policy.json around line 1, the file
is empty and must be replaced with a valid IAM policy JSON (including "Version"
and "Statement"). Fix by creating a proper JSON policy for the intended role:
decide whether this is an EC2/Elastic Beanstalk instance profile or a
CI/CD/deployer role, pick least-privilege vs wildcard scope, and either (A)
reference AWS-managed Elastic Beanstalk policies (e.g.,
AWSElasticBeanstalkWebTier/WorkerTier plus any needed service policies like
AmazonEC2ContainerRegistryReadOnly, CloudWatchLogsFullAccess, or scoped
RDS/ElastiCache permissions) or (B) author a custom policy JSON with a Version
and Statements granting only the required actions (S3 get/put for deployments,
ecs/ecr describe/pull if ECS tasks used, logs:CreateLogStream/PutLogEvents,
ec2/autoscaling describe and tagging, and scoped RDS/ElastiCache access if
needed). Ensure the final file is valid JSON and add comments in PR describing
scope and resource ARNs used for least-privilege.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
aws-infra/README.md (4)
43-44: Fix incomplete sentence; add subject to line 44.Line 44 is missing a subject: "Can be upgraded later..." → who/what can be upgraded? Likely should be "It can be upgraded later..." or "The environment can be upgraded later..."
- Deployment bundle includes: - - `Dockerrun.aws.json` - - `.ebextensions/00_setup.config` - Environment type: - - Single-instance environment (used for development/testing to reduce cost). - - Can be upgraded later to a load-balanced environment for production. + Deployment bundle includes: + - `Dockerrun.aws.json` + - `.ebextensions/00_setup.config` + Environment type: + - Single-instance environment (used for development/testing to reduce cost). + - It can be upgraded later to a load-balanced environment for production.
56-60: Remove redundant phrase on line 59; simplify "originally created" to "created".The phrase "originally created" is redundant in context; "created" alone is sufficient.
- - EB-managed instance security group (default inbound + outbound rules) - - Additional outbound egress security group - *(originally created for App Runner, now reused for EB networking)* + - EB-managed instance security group (default inbound + outbound rules) + - Additional outbound egress security group + *(created for App Runner, now reused for EB networking)*
357-451: Fix markdown issues in deployment workflow section: add language specs to code blocks and remove blank line in blockquote.The extensive blockquote section containing validation instructions has several markdown linting violations:
- Line 358: Blank line inside blockquote (should have no blank lines within blockquote)
- Lines 366, 377, 411, 426, 440: Fenced code blocks missing language specifications (should be
```bash,```json, etc.)Additionally, line 354 uses the adverb "successfully" twice in consecutive bullet points ("connects to Redis successfully" and "schedules run successfully"), which is redundant. Consider replacing one with a synonym or restructuring.
Apply these fixes:
### Step 5 — Validate Deployment - `/api/v2/` returns `200` - Django container remains healthy - - Celery worker connects to Redis successfully - - Celery Beat schedules run successfully + - Celery worker connects to Redis without errors + - Celery Beat schedules tasks as expected - Flower UI loads on port 5555 (if security groups permit) -> > ### How to Validate this deployment? > The points listed above describe the expected state of a successful AWS deployment. > > --- > > ### 1. Confirming that `/api/v2/` returns a successful response > When you open: - > ``` + > ```bash > https://<EB-URL>/api/v2/ > ``` > the browser shows the JSON content returned by Django. > This means that simply opening the URL visually confirms "the API is working," but not the status code. > > To check the actual HTTP status code, use: - > ```bash + > ```bash > curl -I https://<EB-URL>/api/v2/ > ``` > This command returns the HTTP headers. A successful response looks like: - > ``` + > ``` > HTTP/1.1 200 OK > ```For all missing language specs, add the appropriate language identifier after the opening backticks:
```bash,```json, etc.
539-539: Fix markdown emphasis style on final line.Line 539 uses underscore emphasis (
_End of documentation._) instead of the consistent asterisk style used throughout the document. Change to*End of documentation.*for markdown consistency.-_End of documentation._ +*End of documentation.*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aws-infra/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
aws-infra/README.md
[style] ~44-~44: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
[style] ~59-~59: This phrase is redundant. Consider writing “created”.
Context: ... outbound egress security group *(originally created for App Runner, now reused for EB netwo...
(ORIGINALLY_CREATED)
[style] ~270-~270: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...c buckets - Restricting ECR access to only required repositories - Minimizing Cl...
(ADVERB_REPETITION_PREMIUM)
[style] ~354-~354: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...uccessfully - Celery Beat schedules run successfully - Flower UI loads on port 5555 (if secu...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
aws-infra/README.md
358-358: Blank line inside blockquote
(MD028, no-blanks-blockquote)
366-366: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
377-377: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
411-411: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
426-426: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
440-440: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
539-539: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
539-539: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
🔇 Additional comments (3)
aws-infra/README.md (3)
505-535: Comprehensive future improvements roadmap is well-prioritized.The list of recommended enhancements covers critical areas (Secrets Manager, RDS/Redis hardening, IAM least-privilege, CI/CD) and scalability improvements. This aligns well with the earlier acknowledgment in section 5.4 about using broad EB-managed policies as a temporary baseline. Consider adding a priority or phasing note (e.g., "Phase 1: Critical for production" vs. "Phase 2: Scalability enhancements") to help maintainers prioritize implementation order.
229-275: Verify thataws-infra/policies/iam_elastic_beanstalk_policy.jsoncontains an example or template.Section 5.4 documents IAM roles comprehensively and notes "least privilege" recommendations for future hardening. The policy file should include either a concrete, sanitized example of the current EB-managed policy configuration, or a template showing the recommended least-privilege hardened version. If currently empty, maintainers lack a concrete reference artifact to implement or review.
26-31: Include sanitized example versions of the three deployment configuration files referenced in this section.Past review comments requested adding example versions of
.ebextensions/00_setup.config,Dockerrun.aws.json, and.ebignorewith sensitive parts sanitized. Maintainers need concrete, runnable templates for these critical deployment files to understand the expected configuration structure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
aws-infra/README.md (9)
44-44: Clarify incomplete sentence about load-balanced environment upgrade.Line 44 lacks a subject: "Can be upgraded later to a load-balanced environment for production" reads incomplete. Suggest revising to: "The environment can be upgraded later to a load-balanced configuration for production."
59-59: Simplify redundant phrase in security group description.The phrase "originally created for App Runner, now reused" is redundant. Consider simplifying to: "An outbound-only egress security group (reused from prior App Runner setup)."
188-188: Fix emphasis marker formatting on line 188.Markdown linter flags spaces inside emphasis markers. Line 188 reads
** Inbound:**with an extra space after the opening**. Remove the space to comply with markdown formatting.
276-276: Remove redundant adverb in IAM least-privilege section.The adverb "only" is used twice in this sentence: "Restricting S3 access to only specific buckets…Restricting ECR access to only required repositories." Consider removing the second instance or replacing with a synonym like "necessary" for clarity.
365-365: Strengthen the adjective describing CloudFront logging enhancement.The phrase "if deeper monitoring is needed" uses a weak comparative adjective. Consider replacing with: "if comprehensive monitoring is needed" or "if detailed access logs are required."
429-429: Remove redundant adverb in validation checklist.The adverb "successfully" appears twice in quick succession: "Celery worker connects to Redis successfully…Celery Beat schedules run successfully." Consider rewording the second instance to: "Celery Beat schedules are dispatched without errors."
433-433: Remove blank line inside blockquote on line 433.Markdown linter flags a blank line inside the blockquote block starting at line 433. Remove the blank line between the
>markers to maintain proper blockquote formatting.
441-451: Add language specifiers to all fenced code blocks.Several fenced code blocks lack language identifiers, which markdown linters expect:
- Line 441:
```→```bash- Line 452:
```→```bash- Line 486:
```→```text(or omit if showing plaintext output)- Line 501:
```→```text- Line 515:
```httpor```text(HTTP request)Adding language tags improves syntax highlighting and follows markdown best practices.
Also applies to: 452-462, 486-495, 501-510, 515-523
614-614: Update emphasis style to match markdown linter expectations.Markdown linter flags underscore emphasis (
_End of documentation._) on line 614. Replace with asterisks to comply with style guide:*End of documentation.*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aws-infra/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
aws-infra/README.md
[style] ~44-~44: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
[style] ~59-~59: This phrase is redundant. Consider writing “created”.
Context: ... outbound egress security group *(originally created for App Runner, now reused for EB netwo...
(ORIGINALLY_CREATED)
[style] ~276-~276: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...c buckets - Restricting ECR access to only required repositories - Minimizing Cl...
(ADVERB_REPETITION_PREMIUM)
[style] ~365-~365: Consider a different adjective to strengthen your wording.
Context: ...Disabled** (Can be enabled later if deeper monitoring is needed) --- ## 6. .ebe...
(DEEP_PROFOUND)
[style] ~429-~429: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...uccessfully - Celery Beat schedules run successfully - Flower UI loads on port 5555 (if secu...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
aws-infra/README.md
188-188: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
433-433: Blank line inside blockquote
(MD028, no-blanks-blockquote)
441-441: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
452-452: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
486-486: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
501-501: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
515-515: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
614-614: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
614-614: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
🔇 Additional comments (5)
aws-infra/README.md (5)
185-185: RDS public accessibility security issue resolved.Good news: RDS public accessibility is now disabled, which addresses the prior critical security gap flagged in previous reviews. The private subnet configuration and restricted security group (inbound only from EB SG) are properly in place.
398-411: Version numbering guidance clearly documented.The explanation of container versioning (lines 398–411) directly addresses prior feedback requesting clarity on how to choose version tags. This addition is helpful and well-integrated into the deployment workflow.
434-526: Detailed validation section comprehensively addresses prior feedback.The "How to Validate this deployment?" section (lines 434–526) provides clear step-by-step validation instructions with concrete examples (curl commands, log snippets, UI access). This directly addresses the prior reviewer question "how?" and significantly improves usability of the guide.
540-561: Redis TLS configuration location clearly explained.The dedicated subsection explaining where the Redis TLS fix is applied (lines 540–561) directly addresses prior feedback asking "where would I fix this?" The explanation of
.ebextensions/00_setup.configas the source and how EB forwards values to containers is clear and actionable.
26-31: Verify that example configuration files are present or planned.A prior reviewer requested "example versions of these files" (section 2, configuration files) with the response that "sanitized minimal example versions" would be uploaded. The current documentation references these files (
.ebextensions/00_setup.config,Dockerrun.aws.json,.ebignore) but no example snippets or separate example directory is visible in this PR.Please confirm:
- Are sanitized example versions of these configuration files included elsewhere in the PR (e.g., under
aws-infra/examples/)?- If not yet added, should this be tracked as a follow-up task, or should examples be added before merging this PR?
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
aws-infra/images/aws-architecture.pngis excluded by!**/*.png
📒 Files selected for processing (1)
aws-infra/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
aws-infra/README.md
[style] ~49-~49: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
[style] ~280-~280: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...c buckets - Restricting ECR access to only required repositories - Minimizing Cl...
(ADVERB_REPETITION_PREMIUM)
[style] ~369-~369: Consider a different adjective to strengthen your wording.
Context: ...Disabled** (Can be enabled later if deeper monitoring is needed) --- ## 6. .ebe...
(DEEP_PROFOUND)
[style] ~433-~433: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...uccessfully - Celery Beat schedules run successfully - Flower UI loads on port 5555 (if secu...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
aws-infra/README.md
192-192: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
437-437: Blank line inside blockquote
(MD028, no-blanks-blockquote)
445-445: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
456-456: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
490-490: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
505-505: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
519-519: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
615-615: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
615-615: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
🔇 Additional comments (5)
aws-infra/README.md (5)
92-176: Well-organized container and environment variable documentation.The Docker container descriptions and environment variable taxonomy are clear, well-categorized, and easy to navigate. This will help maintainers understand the deployment model and required configuration.
178-302: RDS security concern is properly addressed.The RDS configuration correctly specifies public accessibility as Disabled (line 189) with restrictive inbound rules from only the EB security group. This resolves the prior concern. The overall infrastructure documentation is comprehensive, with RDS and Redis properly isolated in private subnets and secured via security group rules. The IAM section appropriately documents the use of broad managed policies for initial deployment while recommending least-privilege refinement as a future hardening step.
305-371: CloudFront architecture is well-documented and secure.The distribution strategy (S3 for static files, EB for /api/* routes), origin access control, and security posture are clearly explained. The HTTPS enforcement and S3 bucket protection via OAC demonstrate good security practices.
387-427: Deployment workflow is clear and actionable.The five-step deployment process with Docker image versioning guidance is well-structured and includes helpful notes about semantic versioning choices. This makes it easy for contributors to follow a consistent versioning scheme.
532-582: Troubleshooting section thoroughly addresses common deployment issues.The Redis TLS configuration and EB health check sections provide clear fixes with explanations of how Elastic Beanstalk injects environment variables into containers. This directly addresses prior questions about where to apply fixes and demonstrates good understanding of the deployment mechanics.
| - `.ebextensions/00_setup.config` | ||
| - Environment type: | ||
| - Single-instance environment (used for development/testing to reduce cost). | ||
| - Can be upgraded later to a load-balanced environment for production. |
There was a problem hiding this comment.
Fix fragment sentence.
This line lacks a subject. Restructure to: "This environment can be upgraded later to a load-balanced environment for production."
🧰 Tools
🪛 LanguageTool
[style] ~49-~49: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
🤖 Prompt for AI Agents
In aws-infra/README.md around line 49, the sentence "Can be upgraded later to a
load-balanced environment for production." is a fragment without a subject;
replace it with a full sentence such as "This environment can be upgraded later
to a load-balanced environment for production." to fix grammar and clarity.
aws-infra/README.md
Outdated
|
|
||
| ## 9. Future Improvements | ||
|
|
||
| To harden the deployment and move toward a production-grade architecture, the following enhancements are recommended: | ||
|
|
||
| - **Move secrets to AWS Secrets Manager** | ||
| Centralize all sensitive variables (DB password, Redis URL, Django secret key, Sentry key, SendGrid, etc.) and replace `.ebextensions` injection with runtime retrieval. | ||
|
|
||
| - **Enable ElastiCache Multi-AZ + Auto-Failover** | ||
| Improves high availability for Celery and Django caching; eliminates single-node Redis failure risks. | ||
|
|
||
| - **IAM hardening and least-privilege review** | ||
| Replace broad EB-managed policies with reduced IAM policies scoped only to required S3, ECR, CloudWatch, and ECS resources. | ||
|
|
||
| - **Add CI/CD pipeline (GitHub Actions -> ECR -> EB)** | ||
| Automate build, tag, push of images and deployments to Elastic Beanstalk for consistent, reproducible releases. | ||
|
|
||
| - **Add staging environment** | ||
| Separate EB environment (staging) for testing migrations, image builds, and infrastructure changes before production. | ||
|
|
||
| - **Migrate to load-balanced EB environment (optional)** | ||
| Enables rolling deployments, zero-downtime updates, and better scalability. | ||
|
|
||
| - **Enable RDS Multi-AZ + automated backups** | ||
| Ensures database failover and improves disaster recovery readiness. | ||
|
|
||
| - **Add health checks for Celery worker & beat** | ||
| Custom EB or CloudWatch alarms to alert on worker failures, broker connectivity issues, or long task queues. | ||
|
|
||
|
|
||
| --- | ||
|
|
||
| _End of documentation._ |
There was a problem hiding this comment.
Future improvements are well-prioritized and realistic.
The roadmap appropriately covers security hardening (Secrets Manager, IAM least-privilege), reliability enhancements (Multi-AZ, health monitoring), and operational maturity (CI/CD, staging). However, fix the emphasis style on line 615: change _End of documentation._ to *End of documentation.* to match the file's asterisk emphasis convention (MD049).
-_End of documentation._
+*End of documentation.*📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 9. Future Improvements | |
| To harden the deployment and move toward a production-grade architecture, the following enhancements are recommended: | |
| - **Move secrets to AWS Secrets Manager** | |
| Centralize all sensitive variables (DB password, Redis URL, Django secret key, Sentry key, SendGrid, etc.) and replace `.ebextensions` injection with runtime retrieval. | |
| - **Enable ElastiCache Multi-AZ + Auto-Failover** | |
| Improves high availability for Celery and Django caching; eliminates single-node Redis failure risks. | |
| - **IAM hardening and least-privilege review** | |
| Replace broad EB-managed policies with reduced IAM policies scoped only to required S3, ECR, CloudWatch, and ECS resources. | |
| - **Add CI/CD pipeline (GitHub Actions -> ECR -> EB)** | |
| Automate build, tag, push of images and deployments to Elastic Beanstalk for consistent, reproducible releases. | |
| - **Add staging environment** | |
| Separate EB environment (staging) for testing migrations, image builds, and infrastructure changes before production. | |
| - **Migrate to load-balanced EB environment (optional)** | |
| Enables rolling deployments, zero-downtime updates, and better scalability. | |
| - **Enable RDS Multi-AZ + automated backups** | |
| Ensures database failover and improves disaster recovery readiness. | |
| - **Add health checks for Celery worker & beat** | |
| Custom EB or CloudWatch alarms to alert on worker failures, broker connectivity issues, or long task queues. | |
| --- | |
| _End of documentation._ | |
| ## 9. Future Improvements | |
| To harden the deployment and move toward a production-grade architecture, the following enhancements are recommended: | |
| - **Move secrets to AWS Secrets Manager** | |
| Centralize all sensitive variables (DB password, Redis URL, Django secret key, Sentry key, SendGrid, etc.) and replace `.ebextensions` injection with runtime retrieval. | |
| - **Enable ElastiCache Multi-AZ + Auto-Failover** | |
| Improves high availability for Celery and Django caching; eliminates single-node Redis failure risks. | |
| - **IAM hardening and least-privilege review** | |
| Replace broad EB-managed policies with reduced IAM policies scoped only to required S3, ECR, CloudWatch, and ECS resources. | |
| - **Add CI/CD pipeline (GitHub Actions -> ECR -> EB)** | |
| Automate build, tag, push of images and deployments to Elastic Beanstalk for consistent, reproducible releases. | |
| - **Add staging environment** | |
| Separate EB environment (staging) for testing migrations, image builds, and infrastructure changes before production. | |
| - **Migrate to load-balanced EB environment (optional)** | |
| Enables rolling deployments, zero-downtime updates, and better scalability. | |
| - **Enable RDS Multi-AZ + automated backups** | |
| Ensures database failover and improves disaster recovery readiness. | |
| - **Add health checks for Celery worker & beat** | |
| Custom EB or CloudWatch alarms to alert on worker failures, broker connectivity issues, or long task queues. | |
| --- | |
| *End of documentation.* |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
615-615: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
615-615: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
🤖 Prompt for AI Agents
In aws-infra/README.md around lines 583 to 615, the closing emphasis uses
underscores (`_End of documentation._`) which violates the file's asterisk
emphasis convention (MD049); change that line to use asterisks (`*End of
documentation.*`) to match the rest of the document's emphasis style. Ensure
only the delimiter characters are changed and no trailing spaces or other
formatting are altered.
Updated README to include Antenna UI deployment instructions and architecture diagrams.
Updated README to reflect changes in Antenna platform architecture and deployment processes, including backend and frontend components.
Updated README to include AWS CLI in backend services and clarified container count in Elastic Beanstalk deployment. Removed security notes section for a cleaner overview.
bhagyashreewagh
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
aws-infra/README.md (3)
67-67: Fix sentence fragment (previously flagged).This line is still a sentence fragment without a subject. Past review suggested changing to "This environment can be upgraded later to a load-balanced environment for production."
Apply this fix:
- - Can be upgraded later to a load-balanced environment for production. + - This environment can be upgraded later to a load-balanced environment for production.
336-421: Add language identifiers to fenced code blocks (previously flagged).Multiple code blocks in the validation section lack language specifiers, which reduces readability and breaks markdown linting rules.
Add language specs to the code blocks at lines 336, 347, 381, 396, and 410:
When you open: -``` +```bash https://<EB-URL>/api/v2/To check the actual HTTP status code, use:
-bash +bash
curl -I https:///api/v2/This command returns the HTTP headers. A successful response looks like: -``` +```http HTTP/1.1 200 OKApply similar fixes to code blocks at lines 381, 396, and 410 (use `bash` for shell commands, `text` or `log` for log output). --- `438-438`: **Fix markdown formatting: remove trailing space in emphasis.** There's a trailing space inside the bold markers causing malformed markdown. ```diff -- **Security Group (`antenna-rds-sg`) ** +- **Security Group (`antenna-rds-sg`)**
🧹 Nitpick comments (3)
aws-infra/configurations/.ebextensions/00_setup.config_template (1)
27-28: Document the security trade-off of disabling SSL certificate verification.Setting
ssl_cert_reqs=nonedisables certificate verification for Redis connections, which reduces security guarantees (vulnerable to MITM attacks). While this is a known workaround for ElastiCache TLS, it should be documented as a trade-off.Consider adding a comment:
# Redis / Celery - REDIS_URL: "rediss://<REDIS_ENDPOINT>:6379/0?ssl_cert_reqs=none" - CELERY_BROKER_URL: "rediss://<REDIS_ENDPOINT>:6379/0?ssl_cert_reqs=none" + # Note: ssl_cert_reqs=none disables certificate validation (required for ElastiCache) + REDIS_URL: "rediss://<REDIS_ENDPOINT>:6379/0?ssl_cert_reqs=none" + CELERY_BROKER_URL: "rediss://<REDIS_ENDPOINT>:6379/0?ssl_cert_reqs=none"aws-infra/modifications/storage.py (2)
63-69: Narrow the exception type in prefix verification.Catching bare
Exceptionis too broad. While the verification is intentionally non-fatal, catching specific exceptions provides better error visibility.placeholder_key = f"{prefix}/.placeholder" try: s3.write_file(S3_TEST_CONFIG, placeholder_key, b"") logger.info(f"[S3] Verified prefix exists (AWS): {placeholder_key}") - except Exception as e: + except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: logger.warning(f"[S3] Could not verify AWS prefix '{prefix}': {e}")Add the import at the top:
import io import logging +import botocore.exceptions from django.conf import settings
104-109: Clean up unused variable in list comprehension.The variable
iin the list comprehension is unpacked but never used.if skip_existing: keys = s3.list_files(config=config, subdir=subdir, limit=10) - existing_keys = [key.key for key, i in keys if key] + existing_keys = [key.key for key, _ in keys if key] if existing_keys: logger.info(f"Skipping existing images in {subdir}: {existing_keys}") return []Using
_is a Python convention for intentionally unused variables.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
aws-infra/images/aws_architecture_backend.svgis excluded by!**/*.svgaws-infra/images/aws_architecture_frontend.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
aws-infra/README.md(1 hunks)aws-infra/configurations/.ebextensions/00_setup.config_template(1 hunks)aws-infra/configurations/Dockerrun.aws.json_template(1 hunks)aws-infra/modifications/storage.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
aws-infra/modifications/storage.py (3)
ami/main/models.py (1)
S3StorageSource(1390-1488)ami/tests/fixtures/images.py (2)
GeneratedTestFrame(130-138)generate_moth_series(141-239)ami/utils/s3.py (3)
S3Config(34-48)create_bucket(130-140)write_file(553-559)
🪛 LanguageTool
aws-infra/README.md
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...L)**: primary application database. - Amazon ElastiCache (Redis with TLS): Celery ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...**: Celery broker and Django cache. - Amazon S3: object storage (e.g., uploaded fi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e.g., uploaded files/static/media). - Amazon CloudWatch: logs, health monitoring, ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~67-~67: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
[style] ~325-~325: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...uccessfully - Celery Beat schedules run successfully - Flower UI loads on port 5555 (if secu...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
aws-infra/README.md
336-336: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
347-347: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
381-381: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
396-396: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
410-410: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
438-438: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🪛 Ruff (0.14.8)
aws-infra/modifications/storage.py
68-68: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (12)
aws-infra/configurations/.ebextensions/00_setup.config_template (1)
37-43: LGTM! Health check and volume configuration is appropriate.The health check correctly uses HTTP protocol for internal container health checks (HTTPS termination happens at CloudFront/ALB, not within EB), and the 30 GB root volume is reasonable for a single-instance Docker environment.
aws-infra/README.md (5)
1-56: LGTM! Clear and comprehensive architecture overview.The overview effectively documents the multi-container backend architecture and frontend delivery strategy. The separation of backend (EB + ECS) and frontend (S3 + CloudFront) components is well-explained.
110-256: LGTM! Container definitions and IAM configuration are well-documented.The container setup, environment variable taxonomy, and IAM role separation (EC2 instance profile vs. service role) are clearly explained. The note about developers using individual key pairs is good security practice.
473-546: LGTM! Frontend deployment workflow is clear and complete.The S3 upload, CloudFront configuration, and invalidation steps are well-documented. The behavior-based routing (
/api/*→ EB,*→ S3) is correctly described.
548-598: LGTM! Troubleshooting section addresses key deployment issues.The common issues section effectively documents Redis TLS requirements, health check configuration, and migration timing challenges, all of which align with the configuration templates in this PR.
600-632: LGTM! Future improvements are well-prioritized.The roadmap appropriately covers security hardening (Secrets Manager, IAM least-privilege), reliability enhancements (Multi-AZ, health monitoring), and operational maturity (CI/CD, staging). The note about eventually restricting S3 access to CloudFront-only via OAC is good security awareness.
aws-infra/configurations/Dockerrun.aws.json_template (4)
5-23: LGTM! Django container configuration is appropriate.The essential container correctly maps port 80 (host) to 5000 (container), includes necessary environment variables, and links to ML backend services for processing requests.
25-76: LGTM! Celery container configurations are correct.The worker, beat, and flower containers are appropriately marked as non-essential (won't block EB health checks), have reasonable memory allocations, and maintain links to ML backends for job processing.
78-88: LGTM! AWS CLI utility container is configured correctly.The lightweight helper container uses a sleep command to stay alive for ad-hoc AWS operations, which is a standard pattern. The minimal 256 MB memory allocation is appropriate for this use case.
90-110: LGTM! ML backend containers support parallel deployment.Both ML service containers are well-configured with distinct host port mappings (2000 vs. 2003) to avoid conflicts while both internally listen on port 2000. The hostnames match the endpoint URLs configured in other containers.
aws-infra/modifications/storage.py (2)
1-45: LGTM! Dual-mode S3 configuration is clean and well-documented.The environment-aware selection between AWS and MinIO is explicit and well-logged. The configuration correctly constructs endpoint URLs and public base URLs for both modes.
111-134: LGTM! Image generation and upload logic is well-implemented.The function thoughtfully generates timestamped moth images across multiple nights, converts them to JPEG, and uploads to S3 with proper logging. The skip_existing optimization prevents redundant uploads.
| DJANGO_SECRET_KEY: "<DJANGO_SECRET_KEY>" | ||
| DJANGO_DEBUG: "False" | ||
| DJANGO_ALLOWED_HOSTS: "<EB_DOMAIN>,<CLOUDFRONT_DOMAIN>" | ||
| DJANGO_SECURE_SSL_REDIRECT: "True" # disable HTTPS redirect to stop infinite loop |
There was a problem hiding this comment.
Fix contradictory comment on SSL redirect setting.
The comment states "disable HTTPS redirect to stop infinite loop" but the setting DJANGO_SECURE_SSL_REDIRECT is set to "True", which enables HTTPS redirect. Either the comment or the value is incorrect.
If the intent is to enable HTTPS redirect (which is correct for production), update the comment:
- DJANGO_SECURE_SSL_REDIRECT: "True" # disable HTTPS redirect to stop infinite loop
+ DJANGO_SECURE_SSL_REDIRECT: "True" # enable HTTPS redirect for productionIf infinite redirect loops are a concern with health checks, clarify that EB_HEALTHCHECK=1 is the mechanism used to bypass SSL redirect for health check endpoints specifically.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DJANGO_SECURE_SSL_REDIRECT: "True" # disable HTTPS redirect to stop infinite loop | |
| DJANGO_SECURE_SSL_REDIRECT: "True" # enable HTTPS redirect for production |
🤖 Prompt for AI Agents
In aws-infra/configurations/.ebextensions/00_setup.config_template around line
8, the inline comment contradicts the setting: DJANGO_SECURE_SSL_REDIRECT is
"True" (enabling HTTPS redirect) but the comment says "disable HTTPS redirect".
Update the comment to reflect that HTTPS redirect is enabled in production and,
if needed, note that health checks are bypassed via EB_HEALTHCHECK=1 to avoid
infinite redirect loops; alternatively, if the real intent is to disable
redirect, change the value to "False" and keep the existing comment—pick the
correct intent and make the comment/value consistent.
| container_commands: | ||
| 01_run_migrations: | ||
| command: "docker exec $(docker ps -q -f name=django) python manage.py migrate --noinput || true" | ||
| leader_only: true |
There was a problem hiding this comment.
Migration command silently ignores all failures.
The || true suffix causes the migration command to always return success, even if migrations fail due to actual errors (not just timing issues). This masks legitimate migration failures.
Consider a more targeted approach that retries on connection errors but fails on actual migration errors:
container_commands:
01_run_migrations:
- command: "docker exec $(docker ps -q -f name=django) python manage.py migrate --noinput || true"
+ command: |
+ for i in {1..30}; do
+ CONTAINER=$(docker ps -q -f name=django)
+ if [ -n "$CONTAINER" ]; then
+ docker exec $CONTAINER python manage.py migrate --noinput && break
+ fi
+ echo "Waiting for Django container... ($i/30)"
+ sleep 2
+ done
leader_only: trueThis waits for the container to be ready but doesn't mask actual migration errors.
🤖 Prompt for AI Agents
In aws-infra/configurations/.ebextensions/00_setup.config_template around lines
45 to 48, the migration command currently appends "|| true" which masks all
failures; remove the trailing "|| true" and replace with a retry loop that
attempts to run docker exec python manage.py migrate several times with short
sleeps between attempts, but only retries on connection-related failures (e.g.,
container not ready or DB connection errors) and aborts immediately (return
non-zero) on real migration errors; ensure the loop uses a limited retry count
and exits with the migrate command's exit code on final failure so actual
migration errors are not masked.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
aws-infra/modifications/storage.py (3)
18-43: RefineUSE_AWSdetection and annotate intentional MinIO test credentialsThe runtime switching logic looks good, but two small tweaks would make it more robust and linter‑friendly:
USE_AWScurrently checks only for attribute presence, not truthiness. This can select the AWS path even when settings are present but empty, leading to partially configured S3.- The MinIO
minioadmincredentials are clearly non‑secret test values, but they’ll keep triggering S106 unless annotated.You could tighten detection and mark the MinIO creds as intentional, e.g.:
-USE_AWS = all([ - hasattr(settings, "DJANGO_AWS_ACCESS_KEY_ID"), - hasattr(settings, "DJANGO_AWS_SECRET_ACCESS_KEY"), - hasattr(settings, "DJANGO_AWS_STORAGE_BUCKET_NAME"), -]) +USE_AWS = all( + getattr(settings, attr, None) + for attr in ( + "DJANGO_AWS_ACCESS_KEY_ID", + "DJANGO_AWS_SECRET_ACCESS_KEY", + "DJANGO_AWS_STORAGE_BUCKET_NAME", + ) +) @@ - S3_TEST_CONFIG = s3.S3Config( - endpoint_url="http://minio:9000", - access_key_id="minioadmin", - secret_access_key="minioadmin", + S3_TEST_CONFIG = s3.S3Config( + endpoint_url="http://minio:9000", + access_key_id="minioadmin", # noqa: S106 - local MinIO test credentials + secret_access_key="minioadmin", # noqa: S106 - local MinIO test credentialsThis keeps behavior the same in EB while avoiding accidental “AWS mode” when values are blank and silencing the false‑positive secret warning in local/test mode.
52-60: Prefix verification: avoid blindExceptionand log full tracebackThe best‑effort prefix write is a nice safeguard, but the current broad
except Exception as eplus string interpolation both hides the traceback and triggers BLE001/TRY400.If you want to keep this as a non‑fatal catch‑all, consider explicitly documenting that and using
logger.exceptionso failures are debuggable:- try: - s3.write_file(S3_TEST_CONFIG, placeholder_key, b"") - logger.info(f"[S3] Verified prefix exists: {placeholder_key}") - except Exception as e: - logger.error(f"[S3] Could not verify prefix '{prefix}': {e}") + try: + s3.write_file(S3_TEST_CONFIG, placeholder_key, b"") + logger.info("[S3] Verified prefix exists: %s", placeholder_key) + except Exception: # noqa: BLE001 - non-fatal, best-effort prefix check + logger.exception("[S3] Could not verify prefix '%s'", prefix)Alternatively, if you know the concrete exception types from
s3.write_file(e.g., botocore errors), narrowing theexceptclause to those would also satisfy the linter.
80-125: Documentpopulate_bucketbehavior whenskip_existing=TrueThe demo uploader is handy, and the “do nothing if anything already exists under subdir” behavior makes sense for one‑shot seeding. However, that behavior (returning
[]when data already exists) isn’t obvious from the signature alone.To make the API clearer to callers, consider adding a short docstring that spells this out:
def populate_bucket( @@ - skip_existing: bool = True, -) -> list[GeneratedTestFrame]: - - created = [] + skip_existing: bool = True, +) -> list[GeneratedTestFrame]: + """ + Populate the given S3 subdir with generated demo frames. + + Returns a list of frames that were newly uploaded. If ``skip_existing`` is + True and any objects are already present under ``subdir``, the function + logs and returns an empty list without uploading anything. + """ + + created: list[GeneratedTestFrame] = []That should prevent surprises for callers who might otherwise interpret
[]as “no frames generated” rather than “bucket already seeded.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aws-infra/modifications/storage.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
aws-infra/modifications/storage.py (3)
ami/main/models.py (1)
S3StorageSource(1390-1488)ami/tests/fixtures/images.py (2)
GeneratedTestFrame(130-138)generate_moth_series(141-239)ami/utils/s3.py (2)
S3Config(34-48)write_file(553-559)
🪛 Ruff (0.14.8)
aws-infra/modifications/storage.py
39-39: Possible hardcoded password assigned to argument: "secret_access_key"
(S106)
58-58: Do not catch blind exception: Exception
(BLE001)
59-59: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
carlosgjs
left a comment
carlosgjs
left a comment
There was a problem hiding this comment.
just some minor comments, thx!
|
|
||
| This document describes the AWS infrastructure and deployment pipeline for the Antenna backend. | ||
| The system runs on AWS Elastic Beanstalk (ECS-based multicontainer) using Docker, Celery, ElastiCache Redis (TLS), RDS PostgreSQL, S3, ECR, and Sentry. | ||
| This document describes the AWS infrastructure and deployment pipeline for the Antenna platform. |
There was a problem hiding this comment.
Can you add a note that AWS deployment is experimental and not supported at this time?
| from ami.utils import s3 | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
There was a problem hiding this comment.
Can you add a comment about the purpose of this module and when/where the two functions run?
| ## 8. Future Improvements | ||
|
|
||
| To harden the deployment and move toward a production-grade architecture, the following enhancements are recommended: | ||
|
|
There was a problem hiding this comment.
I would add moving the secrets into AWS Secrets manager so that they don't have to be set in plain text in the config files
There was a problem hiding this comment.
Yes I already mentioned it
Added a warning about the experimental nature of the AWS deployment.
👷 Deploy request for antenna-ssec pending review.Visit the deploys page to approve it
|
2 participants
This PR adds end-to-end AWS deployment and infrastructure documentation for the Antenna platform, along with safe templates for deployment configuration and a small test fixture update to support S3 vs MinIO.
Closes: #967
What’s Included
Documentation
New
aws-infra/README.mdcovering the full Antenna platform deployment:/api/*proxying to EBDeployment templates (no secrets)
.ebextensions/00_setup.configtemplate with placeholders (no credentials committed)Dockerrun.aws.jsontemplate with placeholders for account/region/image tags and runtime configCode change
Updated
ami/tests/fixtures/storage.pyto support:DJANGO_AWS_*settings are present (Elastic Beanstalk/prod-like env)Why This Matters
The project previously lacked cohesive, end-to-end infrastructure documentation. These additions make it easier for maintainers and contributors to reproduce the deployment, understand AWS resource relationships, and safely operate/update the platform.
Notes
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.