Skip to content
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

#938 - expand healthchecks to all preprocessors, services, handlers, and the orchestrator #945

Conversation

shahdyousefak
Copy link
Contributor

@shahdyousefak shahdyousefak commented Jan 21, 2025

Please ensure you've followed the checklist and provide all the required information before requesting a review.
If you do not have everything applicable to your PR, it will not be reviewed!
If you don't know what something is or if it applies to you, ask!

Don't delete below this line.


Required Information

  • I referenced the issue addressed in this PR. --> Added basic healthchecks to all preprocessors, services, handlers, and the orchestrator.
  • I described the changes made and how these address the issue.
  • I described how I tested these changes. --> adding them to the override, and observing its health status turn healthy. I've also corrupted the health endpoint, and when ./healtcheck.sh ran, it returns unhealthy, and pings #automated_tests. In fact, this PR is a good test itself, since if any context/curl installation/endpoint failures occurred, it would fail the respective test.

Coding/Commit Requirements

  • I followed applicable coding standards where appropriate (e.g., PEP8)
  • I have not committed any models or other large files.

New Component Checklist (mandatory for new microservices)

  • I added an entry to docker-compose.yml and build.yml.
  • I created A CI workflow under .github/workflows.
  • I have created a README.md file that describes what the component does and what it depends on (other microservices, ML models, etc.).

OR

  • I have not added a new component in this PR.

@shahdyousefak shahdyousefak linked an issue Jan 21, 2025 that may be closed by this pull request
@shahdyousefak shahdyousefak changed the title 938 expand healthchecks to all preprocessors services handlers #938 - expand healthchecks to all preprocessors services handlers Jan 22, 2025
@shahdyousefak shahdyousefak changed the title #938 - expand healthchecks to all preprocessors services handlers #938 - expand healthchecks to all preprocessors, services, handlers, and the orchestrator Jan 22, 2025
@shahdyousefak shahdyousefak removed their assignment Jan 23, 2025
@jaydeepsingh25
Copy link
Contributor

Can we please have same timestamp format across all the files?
In python you can use isoformat() function available in datetime module.
Other than that, changes look good to me

@shahdyousefak
Copy link
Contributor Author

Can we please have same timestamp format across all the files? In python you can use isoformat() function available in datetime module. Other than that, changes look good to me

thanks, I implemented this!

@@ -2,4 +2,9 @@

/usr/bin/pipewire &

#healthcheck
while :; do
echo -e "HTTP/1.1 200 OK\nContent-Type: application/json\n\n{\"status\": \"healthy\"}" | nc -l -p 57110
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont we need a break here so that it exits in case of failure?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, it only occurred to me yesterday. thanks for catching this, added.

@jaydeepsingh25 jaydeepsingh25 merged commit 69f2162 into main Jan 28, 2025
82 checks passed
@jaydeepsingh25 jaydeepsingh25 deleted the 938-expand-healthchecks-to-all-preprocessors-services-handlers branch January 28, 2025 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expand Healthchecks to all Preprocessors, Services, Handlers
2 participants