-
Notifications
You must be signed in to change notification settings - Fork 44
refactor: simplify error handling in GCP Infrastructure Manager deploy script #3865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This pull request does not have a backport label. Could you fix it @amirbenun? 🙏
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the error handling in the GCP Elastic Agent Infrastructure Manager deployment script to use a more idiomatic shell scripting pattern. The change simplifies the code by replacing explicit exit code capture with a direct if ! conditional pattern.
Changes:
- Replaced explicit exit code capture (
EXIT_CODE=$?) with inlineif !pattern for the gcloud deployment command - Simplified error message by removing specific exit code output
- Normalized exit behavior to always return code 1 on deployment failure
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --input-values="${INPUT_VALUES}"; then | ||
| echo "" | ||
| echo "Deployment failed with exit code $EXIT_CODE" | ||
| echo "Deployment failed" |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message no longer includes the specific exit code from the gcloud command. While the refactored pattern is more idiomatic, the exit code can provide valuable debugging information. Consider preserving this information by capturing the exit code in a variable within the if block or including it in the error message using $? immediately after the command fails.
| echo " gcloud compute instances get-serial-port-output elastic-agent-vm-${RESOURCE_SUFFIX} --zone=${EFFECTIVE_ZONE} --project=${PROJECT_ID}" | ||
| echo "" | ||
| exit $EXIT_CODE | ||
| exit 1 |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exit code is now hardcoded to 1, which loses the original exit code from the gcloud command. This could impact automated systems or scripts that depend on specific exit codes to determine the type of failure. Consider using 'exit $?' immediately after the if condition to preserve the original exit code, or document why normalizing to exit code 1 is preferred.
…y script (elastic#3865) Refactors the error handling in the GCP Elastic Agent Infrastructure Manager deployment script to use a more idiomatic shell pattern. The change replaces the explicit exit code capture and conditional check with a direct if-not pattern for the gcloud command, making the script more readable and following shell scripting best practices. (cherry picked from commit 59c61ae)
…y script (elastic#3865) Refactors the error handling in the GCP Elastic Agent Infrastructure Manager deployment script to use a more idiomatic shell pattern. The change replaces the explicit exit code capture and conditional check with a direct if-not pattern for the gcloud command, making the script more readable and following shell scripting best practices. (cherry picked from commit 59c61ae)
…y script (elastic#3865) Refactors the error handling in the GCP Elastic Agent Infrastructure Manager deployment script to use a more idiomatic shell pattern. The change replaces the explicit exit code capture and conditional check with a direct if-not pattern for the gcloud command, making the script more readable and following shell scripting best practices. (cherry picked from commit 59c61ae)
Refactors the error handling in the GCP Elastic Agent Infrastructure Manager deployment script to use a more idiomatic shell pattern. The change replaces the explicit exit code capture and conditional check with a direct if-not pattern for the gcloud command, making the script more readable and following shell scripting best practices.