feature: Add more logs during deployments and scaling#246
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds logging statements to the deployment scaling loop in src/jobs/helm_aws.yml to output the targeted deployment and the target replica count. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull Request Overview
The PR successfully introduces descriptive logging for Helm/AWS deployment and scaling operations, which will improve observability. However, the implementation in src/jobs/helm_aws.yml contains a logic inefficiency: it executes helm get manifest repeatedly inside a loop, which can result in unnecessary overhead and potential Kubernetes API throttling.
Furthermore, there is a lack of validation for the REPLICA_COUNT variable; if the manifest retrieval fails or returns unexpected data, the scaling command will fail with a syntax error. Despite the Codacy grade being up to standards, these robustness and performance issues should be addressed. Finally, the PR lacks automated test scenarios to verify that the newly added logs are correctly formatted and emitted during different execution paths.
Test suggestions
- Verify that targeting logs (deployment, release, namespace) are printed to stdout during the job execution.
- Verify that scaling logs (deployment name, replica count) are printed correctly for both scale-down and reset paths.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that targeting logs (deployment, release, namespace) are printed to stdout during the job execution.
2. Verify that scaling logs (deployment name, replica count) are printed correctly for both scale-down and reset paths.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| if [ << parameters.scale_environment_down >> = true ]; then | ||
| REPLICA_COUNT=0 | ||
| else | ||
| REPLICA_COUNT=$(helm get manifest -n $NAMESPACE $RELEASE | yq eval -o json | jq -r ". | select( (.kind == \"Deployment\") and (.metadata.name ==\"$DEPLOYMENT\") ) | .spec.replicas" ) |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: The helm get manifest command is called repeatedly inside the loop. To improve performance and robustness, consider fetching the manifest once per release and storing it in a variable. Additionally, you should verify that REPLICA_COUNT is a valid integer before executing the kubectl scale command to prevent runtime errors (e.g., syntax errors if the variable is empty).
Adding logs to avoid misleading logs. Usually before something blowing up, you have the name of the previous deployment