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

Timeouts too long or too short #430

Closed
sssoleileraaa opened this issue Jun 18, 2019 · 3 comments · Fixed by #515
Closed

Timeouts too long or too short #430

sssoleileraaa opened this issue Jun 18, 2019 · 3 comments · Fixed by #515
Assignees

Comments

@sssoleileraaa
Copy link
Contributor

sssoleileraaa commented Jun 18, 2019

Description

The client used to pass a timeout value to the SDK to tell it that a download_submission and download_reply request should only take 20 seconds. Now the client no longer passes timeout values and instead relies on the SDK to determine when a timeout occurs with the server.

Currently, the SDK only uses the following default values to determine when there is a timeout:

DEFAULT_REQUEST_TIMEOUT = 20  # 20 seconds
DEFAULT_DOWNLOAD_TIMEOUT = 60 * 60  # 60 minutes

The problem is that 60 minutes is too long for smaller message and document files. And maybe it's not long enough for larger files. It also doesn't quite makes sense to have a vastly different timeout default between text replies and text messages. A 20 second timeout for a reply is so much smaller than a 60 minute timeout for a message. They really should be using the same timeout defaults.

Ideas

  • The SDK could calculate a minimum bitrate for downloads based on file size
  • This SDK could also use a ramp-up policy that increases the timeout size based on number of retries.
@sssoleileraaa
Copy link
Contributor Author

I dont think I should have closed this

@sssoleileraaa sssoleileraaa reopened this Dec 3, 2019
@redshiftzero
Copy link
Contributor

we should close this in favor of #648 @creviera ?

@eloquence eloquence modified the milestones: 0.2.0alpha, 0.2.0beta Dec 12, 2019
@eloquence
Copy link
Member

Closing in favor of #648 per above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants