Skip to content

LocalBackend._monitor_openai_server improvement #388

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 28, 2025
Merged

LocalBackend._monitor_openai_server improvement #388

merged 2 commits into from
Sep 28, 2025
+6 −4

Conversation

@giladfrid009
Copy link
Contributor

  • Do not add additional load the server while performing health-check by sending inference requests; rather check if the actual model is loaded on the server via openai_client.models.retrieve
  • added configurable timeout for the health check via env variable "ART_SERVER_MONITOR_TIMEOUT"

@giladfrid009
LocalBackend.__monitor_openai_server():
93eebeb 
- do not load the server while performing health-check with additional inference requests; rather check if the actual model is loaded on the server
-  added configurable timeout for the health check
@giladfrid009
linting
43400c0
@giladfrid009 giladfrid009 changed the title local backend. monitor OpenAI server() improvement LocalBackend._monitor_openai_server improvement Sep 2, 2025
@giladfrid009
Copy link
Contributor Author

I added the ART_SERVER_MONITOR_TIMEOUT option since I encountered several times unexplained timeouts from this function, and ended up completely commenting it out in my source. I think ART_SERVER_MONITOR_TIMEOUT might be a good middle-point (although it introduces additional env variable).

Possible modification:ART_SERVER_MONITOR_TIMEOUT=-1 or overall negative values disable the timeout altogether.

@giladfrid009
Copy link
Contributor Author

@bradhilton requesting review :)

bradhilton
bradhilton approved these changes Sep 28, 2025
View reviewed changes
Copy link
Collaborator

@bradhilton bradhilton left a comment

Choose a reason for hiding this comment

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

LGTM

@bradhilton bradhilton merged commit 78bfef1 into OpenPipe:main Sep 28, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bradhilton bradhilton bradhilton approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@giladfrid009 @bradhilton