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

feat!: add default connect and read timeout #218

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paalbra
Copy link
Contributor

@paalbra paalbra commented Sep 26, 2023

I've experienced that the Zabbix API sometimes hang. I believe this is due to some database related issues (hang on write operations only).

If someone encounter this problem a script using pyzabbix might hang forever if you forget to add a timeout parameter:

pyzabbix/pyzabbix/api.py

Lines 61 to 64 in 962693d

:param timeout: optional connect and read timeout in seconds, default: None
If you're using Requests >= 2.4 you can set it as
tuple: "(connect, read)" which is used to set individual
connect and read timeouts.

The requests quickstart says the following about the timeout parameter:

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

Following this guide I think that pyzabbix should have some kind of default, rather than None.

Also quoting from the quickstart:

timeout is not a time limit on the entire response download; rather, an exception is raised if the server has not issued a response for timeout seconds (more precisely, if no bytes have been received on the underlying socket for timeout seconds). If no timeout is specified explicitly, requests do not time out.

I think that a timeout of 30 seconds will be a very, very conservative default, but it should prevent that someone that forget to set a timeout encounter scripts that hang for ever.

@jooola
Copy link
Collaborator

jooola commented Oct 3, 2023

This would be a breaking change and should be part of a v2.

@jooola jooola changed the title Add a conservative default connect/read timeout feat!: add default connect and read timeout Dec 18, 2023
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