-
Notifications
You must be signed in to change notification settings - Fork 174
Rewrite api health check script #4807
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
base: main
Are you sure you want to change the base?
Conversation
Unit Test Results0 tests 0 ✅ 0s ⏱️ Results for commit e51b549. ♻️ This comment has been updated with latest results. |
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 rewrites the API health check script to improve efficiency in CI/CD pipelines by replacing variable sleep intervals (starting at 180 seconds) with fixed 10-second intervals and a 7-minute maximum timeout.
Changes:
- Replaced retry logic with fixed 10-second check intervals and 7-minute maximum timeout
- Refactored code into dedicated functions for better separation of concerns
- Enhanced output messages with clearer progress indicators and elapsed time tracking
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| devops/scripts/api_healthcheck.sh | Complete rewrite of health check logic with fixed intervals, better structure, and improved output |
| CHANGELOG.md | Added entry documenting the script improvements |
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
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
marrobi
left a comment
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.
If it works, LGTM! Is an outstanding copilot comment...
dd78080 to
e7ef3eb
Compare
Co-authored-by: Copilot <[email protected]>
|
/test |
|
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/21109558672 (with refid (in response to this comment from @tamirkamara) |
1 similar comment
|
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/21109558672 (with refid (in response to this comment from @tamirkamara) |
What is being addressed
The api healthcheck script is mainly used in cicd to test if it's healthy before starting sending requests to it. The current method isn't efficient as it sleep time between attempts starts with high numbers (180 secs) and is reduced as attempts are made, the result is several minutes are waisted on cicd runs I've seen recently.
Another issue was that the script didn't recognize some situations correctly and first attempts on new environments are likely to produce a failure. I'm not sure why exactly but this new version seem to work better.
How is this addressed