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

Include a timeout for session.post in sync_execute #46

Open
mmangus opened this issue Apr 12, 2024 · 0 comments
Open

Include a timeout for session.post in sync_execute #46

mmangus opened this issue Apr 12, 2024 · 0 comments

Comments

@mmangus
Copy link

mmangus commented Apr 12, 2024

In sync_execute, commands are executed by making requests to the Upstash REST API like this...

response = session.post(url, headers=headers, json=command).json()

...where session is a requests.Session instance.

As indicated by the requests documentation, this call should include a timeout to avoid blocking for long periods in the event of networking trouble:

Nearly all production code should use this parameter [timeout] in nearly all requests. Failure to do so can cause your program to hang indefinitely

For the Redis workloads I typically have, no response from Redis is better than a slow response from Redis. The client should at least allow users to configure a timeout for these requests, and in my opinion should also have a default timeout enabled without user intervention.

@mmangus mmangus changed the title Include a timeout for _session.post Include a timeout for session.post in sync_execute Apr 12, 2024
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

No branches or pull requests

1 participant