Conversation
Signed-off-by: sahil.kamble@ayanworks.com <sahil.kamble@ayanworks.com>
Signed-off-by: sahil.kamble@ayanworks.com <sahil.kamble@ayanworks.com>
Signed-off-by: sahil.kamble@ayanworks.com <sahil.kamble@ayanworks.com>
Signed-off-by: sahil.kamble@ayanworks.com <sahil.kamble@ayanworks.com>
WalkthroughFour AFJ deployment scripts are updated to create the endpoints directory before writing files, increasing timeout thresholds from 6 to 20 iterations, and removing task ID extraction logic in favor of simplified endpoint JSON generation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/agent-provisioning/AFJ/scripts/docker_start_agent.sh (2)
229-241: Critical: Path mismatch between mkdir and file write location.Line 229 sets
ENDPOINT="${PWD}/endpoints/..."but line 236 creates the directory at${PWD}/agent-provisioning/AFJ/endpoints. The file write on line 237 will fail because the parent directory doesn't exist at the location where ENDPOINT points.Fix the path inconsistency:
- ENDPOINT="${PWD}/endpoints/${AGENCY}_${CONTAINER_NAME}.json" + ENDPOINT="${PWD}/agent-provisioning/AFJ/endpoints/${AGENCY}_${CONTAINER_NAME}.json"Alternatively, update line 236 to match the ENDPOINT variable (though the longer path is consistent with other scripts in the PR).
92-104: Misleading log messages for directory creation.The error messages are identical for both the endpoints and agent-config directory checks (lines 92-104). Additionally, line 100 refers to "Endpoints directory" when checking the agent-config directory. This creates confusion during debugging.
Improve clarity:
if [ -d "${PWD}/agent-provisioning/AFJ/endpoints" ]; then echo "Endpoints directory exists." else echo "Error: Endpoints directory does not exist. Creating..." - mkdir -p ${PWD}/agent-provisioning/AFJ/endpoints + mkdir -p "${PWD}/agent-provisioning/AFJ/endpoints" fi if [ -d "${PWD}/agent-provisioning/AFJ/agent-config" ]; then - echo "Endpoints directory exists." + echo "Agent config directory exists." else - echo "Error: Endpoints directory does not exists." - mkdir -p ${PWD}/agent-provisioning/AFJ/agent-config + echo "Error: Agent config directory does not exist. Creating..." + mkdir -p "${PWD}/agent-provisioning/AFJ/agent-config" fiAlso note: using
mkdir -pmeans the error case never actually errors—consider whether this is the intended behavior.apps/agent-provisioning/AFJ/scripts/start_agent.sh (1)
230-244: Critical: Path mismatch between mkdir and file write location.Line 232 sets
ENDPOINT="${PWD}/endpoints/..."but line 239 creates the directory at${PWD}/agent-provisioning/AFJ/endpoints. The file write on line 240 will fail because the parent directory doesn't exist at the location where ENDPOINT points.Fix the path inconsistency:
- ENDPOINT="${PWD}/endpoints/${AGENCY}_${CONTAINER_NAME}.json" + ENDPOINT="${PWD}/agent-provisioning/AFJ/endpoints/${AGENCY}_${CONTAINER_NAME}.json"This aligns with the directory created on line 239 and matches the pattern in
fargate.shandstart_agent_ecs.sh.apps/agent-provisioning/AFJ/scripts/fargate.sh (1)
9-9: Syntax error in parameter assignment.Line 9 contains an invalid assignment:
RANDOM_SEED=$5ashould beRANDOM_SEED=$5. The trailing "a" breaks the variable binding.- RANDOM_SEED=$5a + RANDOM_SEED=$5
🧹 Nitpick comments (1)
apps/agent-provisioning/AFJ/scripts/start_agent.sh (1)
231-231: Consider removing unused endpoint file logic.The inline comment on line 231 acknowledges that the endpoint file "is not actually being read." If true, the entire endpoint generation block (lines 231–244) could be removed to reduce maintenance burden.
Would you like me to help identify and remove unused endpoint file generation logic across all scripts in this PR?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/agent-provisioning/AFJ/scripts/docker_start_agent.sh(1 hunks)apps/agent-provisioning/AFJ/scripts/fargate.sh(2 hunks)apps/agent-provisioning/AFJ/scripts/start_agent.sh(1 hunks)apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh(2 hunks)
🔇 Additional comments (3)
apps/agent-provisioning/AFJ/scripts/fargate.sh (2)
315-336: Verify intent of timeout threshold increase.Line 318 increases the agent readiness check loop from 6 to 20 iterations (60s → 200s maximum wait). This change appears consistent across ECS deployment scripts but extends startup duration significantly.
Please confirm:
- Was this change driven by observed agent startup delays in testing?
- Is a 3.3x increase in timeout acceptable for deployment SLAs?
- Should this be configurable as a parameter?
342-348: Endpoint directory creation is correctly implemented.The path handling is consistent: directory creation (line 343) and file write (line 344) both reference
${PWD}/agent-provisioning/AFJ/endpoints. This pattern should be matched in the other scripts.apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh (1)
328-334: Endpoint directory and file handling is correctly implemented.The path handling is consistent: directory creation (line 329) and file write (line 330) both reference
${PWD}/agent-provisioning/AFJ/endpoints. The pattern matchesfargate.sh.
2 participants
What ?
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.