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

Configure read timeout based on wait parameter #373

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Conversation

aron
Copy link
Contributor

@aron aron commented Oct 15, 2024

This fixes a bug in the new wait implementation where the default read timeout for the HTTP client is shorter than the timeout on the server. This results in the client erroring before the server has had the opportunity to respond with a partial prediction.

This commit now provides a custom timeout for the predictions.create request based on the wait parameter provided. We add a 500ms buffer to the timeout to account for some discrepancy between server and client timings.

I attempted to try and refactor the shared code between models, deployments & predictions but gave up. We now have a single function that creates the headers and timeout params and passes them in at the various call sites.

@aron aron force-pushed the fix-read-timeout branch from fbc1713 to 8a3e961 Compare October 15, 2024 11:23
This fixes a bug in the new `wait` implementation where the default read
timeout for the HTTP client is shorter than the timeout on the server.
This results in the client erroring before the server has had the
oppertunity to respond with a partial prediction.

This commit now provides a custom timeout for the `predictions.create`
request based on the `wait` parameter provided. We add a 500ms buffer
to the timeout to account for some discrepancy between server and
client timings.
@aron aron force-pushed the fix-read-timeout branch from 8a3e961 to d5a7ac2 Compare October 15, 2024 11:24
@aron aron merged commit 7cfd984 into main Oct 16, 2024
7 checks passed
@aron aron deleted the fix-read-timeout branch October 16, 2024 14:40
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.

2 participants