Clean up connect_timeout handling

Removes the unintentional incompatible change between 1.7.1 and 1.8.0.

Fixes #127

CHANGES.md

@@ -1,14 +1,14 @@
 # unreleased
 
-new features since 1.8.1
+New features since 1.8.1
 
 * Tell the server more about the connecting client: hostname,
   application name, pymonetdb version, process id and an optional remark.
   This information will show up in the `sys.sessions` table.
   Configurable with the new settings `client_info`, `client_application`
   and `client_remark`.
 
-bug fixes
+Bug fixes
 
 * Use the right directory when scanning for Unix Domain sockets
 
@@ -18,6 +18,10 @@ bug fixes
 * Support result set format of PREPARE statements on older MonetDB
   versions.
 
+* Allow connect_timeout=-1 again, the way it was before 1.8.0.
+  However, avoid setting the socket to non-blocking mode.
+  See [Issue #127](https://github.com/MonetDB/pymonetdb/issues/127).
+
 # 1.8.1
 
 changes since 1.8.0

pymonetdb/__init__.py

@@ -99,7 +99,9 @@ def connect(    # noqa C901
     user : str
         alias for username
     connect_timeout : float
-        socket timeout when connecting, in seconds
+        socket timeout when connecting, in seconds, 0=block indefinitely,
+        -1=system default which usually also is 'block indefinitely'
+        (default: -1)
     binary : int
         enable binary result sets when possible if > 0 (default: 1)
     replysize : str

pymonetdb/mapi.py

@@ -193,11 +193,6 @@ def connect_loop(self):
                 self.try_connect()
                 assert self.socket is not None
 
-                if self.target.connect_timeout:
-                    # The new socket's timeout was overridden during the
-                    # connect. Put it back.
-                    self.socket.settimeout(socket.getdefaulttimeout())
-
                 # Once connected, deal with the file handle passing protocol,
                 # AND with TLS. Note that these are necessarily exclusive, we
                 # can't do TLS over unix domain sockets.
@@ -228,17 +223,32 @@ def connect_loop(self):
 
     def try_connect(self):  # noqa C901
         err = None
-        timeout = self.target.connect_timeout
+
+        default_timeout = socket.getdefaulttimeout()
+        if default_timeout == 0:
+            raise ProgrammingError('Global socket timeout set to non-blocking, pymonetdb does not support that')
+        t = self.target.connect_timeout
+        if t == -1:
+            # This is the only negative value allowed by Target.validate().
+            # It means 'leave it alone'
+            set_timeout = False
+        else:
+            set_timeout = True
+            # Our settings and and socket.settimeout() assign different meanings to
+            # value '0'
+            custom_timeout = None if t == 0 else t
 
         sock = self.target.connect_unix
         if sock and hasattr(socket, 'AF_UNIX'):
             s = socket.socket(socket.AF_UNIX)
-            if timeout:
-                s.settimeout(float(timeout))
             try:
+                if set_timeout:
+                    s.settimeout(custom_timeout)
                 s.connect(sock)
                 # it worked!
                 logger.debug(f"Connected to {sock}")
+                if set_timeout:
+                    s.settimeout(default_timeout)
                 self.socket = s
                 self.is_tcp = False
                 return
@@ -252,14 +262,16 @@ def try_connect(self):  # noqa C901
             addrs = socket.getaddrinfo(host, port, socket.AF_UNSPEC, socket.SOCK_STREAM)
             for fam, typ, proto, cname, addr in addrs:
                 s = socket.socket(fam, typ, proto)
-                s.setsockopt(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)
-                s.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
-                if timeout:
-                    s.settimeout(float(timeout))
                 try:
+                    if set_timeout:
+                        s.settimeout(custom_timeout)
+                    s.setsockopt(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)
+                    s.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
                     s.connect(addr)
                     # it worked!
                     logger.debug(f"Connected to {addr[0]} port {addr[1]}")
+                    if set_timeout:
+                        s.settimeout(default_timeout)
                     self.socket = s
                     self.is_tcp = True
                     return

pymonetdb/target.py

@@ -73,7 +73,7 @@ def looks_like_url(text: str) -> bool:
     replysize=None,
     fetchsize=None,
     maxprefetch=None,
-    connect_timeout=0,
+    connect_timeout=-1,
     client_info=True,
     client_application="",
     client_remark="",
@@ -104,6 +104,8 @@ def __init__(self, name, typ, doc):
             self.parser = int
         elif typ == 'bool':
             self.parser = parse_bool
+        elif typ == 'float':
+            self.parser = float
         else:
             raise ValueError(f"invalid type '{typ}'")
         self.__doc__ = doc
@@ -175,7 +177,7 @@ def clone(self):
     replysize = urlparam('replysize', 'integer',
                          'rows beyond this limit are retrieved on demand, <1 means unlimited')
     maxprefetch = urlparam('maxprefetch', 'integer', 'specific to pymonetdb')
-    connect_timeout = urlparam('connect_timeout', 'integer', 'abort if connect takes longer than this')
+    connect_timeout = urlparam('connect_timeout', 'float', 'abort if connect takes longer than this; 0=block indefinitely; -1=system default')
     client_info = urlparam('client_info', 'bool', 'whether to send client details when connecting')
     client_application = urlparam('client_application', 'string', 'application name to send in client details')
     client_remark = urlparam('client_remark', 'string', 'application name to send in client details')
@@ -431,6 +433,10 @@ def validate(self):    # noqa C901
         if self.clientcert and not self.clientkey:
             raise ValueError("clientcert can only be used together with clientkey")
 
+        # 10. pymonetdb-specific
+        if self.connect_timeout < 0 and self.connect_timeout != -1:
+            raise ValueError("connection_timeout must either be >= 0 or -1")
+
     @property
     def connect_scan(self):
         if not self.database: