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

Handling of connect_timeout parameter has changed, needs further improvements #127

Closed
joerivanruth opened this issue Aug 26, 2024 · 1 comment
Assignees

Comments

@joerivanruth
Copy link
Member

Before pymonetdb 1.8.0, the value of connection_timeout could be one of:

  • -1 (the default), to use the Python-wide default timeout,
  • None, to block indefinitely,
  • 0, to enable non-blocking mode on the socket,
  • >0, to set a timeout.

All negative values except -1 yielded an error message.

Note however that pymonetdb was not designed for use with non-blocking sockets. This means setting it to zero, either directly or using socket.setdefaulttimeout(), has never worked.

In 1.8.0, the behaviour was changed in a not well thought out manner. The default changed to 0 and the values 0 and None both meant "use the system default". Other values, including -1 were passed as-is to socket.settimeout(). However, that function does not accept negative values.

It turns out some users explicitly pass connection_timeout=-1. This now fails, which is undesirable. However, the old behaviour is also suboptimal because value 0 is unusable. It makes more sense to interpret connection_timeout=0 as an explicit request for blocking without timeout.

The following table lists the original, the modified and the desired behaviour:

1.7.1 1.8.1 DESIRED
unspecified sysdefault sysdefault sysdefault but not non-blocking
-1 sysdefault error sysdefault but not non-blocking
other negative error error error
0 non-blocking (broken) sysdefault blocking
>0 timeout timeout timeout
@joerivanruth joerivanruth self-assigned this Aug 26, 2024
@MonetDB MonetDB deleted a comment Aug 26, 2024
joerivanruth added a commit that referenced this issue Aug 27, 2024
Removes the unintentional incompatible change between 1.7.1 and 1.8.0.

Fixes #127
@joerivanruth joerivanruth reopened this Aug 27, 2024
@joerivanruth
Copy link
Member Author

Tests have been added, issue can be now be closed.

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