Separate connect/read timeouts in pylinkahead.ini

Summary

Addresses https://gitlab.indiscale.com/caosdb/customers/f-fit/management/-/issues/93 / https://gitlab.com/linkahead/linkahead-pylib/-/issues/127
It is now possible to set separate connect/read timeouts or timeout None in pylinkahead.ini.

Corresponding Test-MR: caosdb-pyinttest!86 (merged)

Focus

• Is the CustomValidator an acceptable solution to the validation of tuples as array?
• Are there any cornercases where this breaks a request that I missed?

Test Environment

Manual Testing

Check List for the Author

• All automated tests pass
• Reference related issues
• Up-to-date CHANGELOG.md (or not necessary)
• Up-to-date JSON schema (or not necessary)
• Appropriate user and developer documentation (or not necessary)
• Annotations in code (Gitlab comments)

Check List for the Reviewer

• I understand the intent of this MR
• All automated tests pass
• Up-to-date CHANGELOG.md (or not necessary)
• Appropriate user and developer documentation (or not necessary)
• The test environment setup works and the intended behavior is reproducible in the test environment
• In-code documentation and comments are up-to-date.
• Check: Are there specifications? Are they satisfied?

src/linkahead/connection/connection.py

@@ -39,7 +39,7 @@ from requests.adapters import HTTPAdapter
 from requests.exceptions import ConnectionError as HTTPConnectionError
 from urllib3.poolmanager import PoolManager
 
-from ..configuration import get_config
+from ..configuration import get_config, config_to_yaml
 from ..exceptions import (ConfigurationError, HTTPClientError,
                           HTTPForbiddenError, HTTPResourceNotFoundError,
                           HTTPServerError, HTTPURITooLongError,
@@ -422,8 +422,10 @@ def configure_connection(**kwargs):
         - "keyring"  Uses the `keyring` library.
         - "auth_token" Uses only a given auth_token.
 
-    timeout : int
+    timeout : int, tuple, or None
         A connection timeout in seconds. (Default: 210)
+        If a tuple is given, they are used as connect and read timeouts
+        respectively, timeout None disables the timeout.
 
     ssl_insecure : bool
         Whether SSL certificate warnings should be ignored. Only use this for
@@ -465,21 +467,29 @@ def configure_connection(**kwargs):
     global_conf = {}
     conf = get_config()
     # Convert config to dict, with preserving types
-    int_opts = ["timeout"]
+    int_opts = []
     bool_opts = ["ssl_insecure"]
+    other_opts = ["timeout"]
 
     if conf.has_section("Connection"):
         global_conf = dict(conf.items("Connection"))
-        # Integer options
 
+        # Integer options
         for opt in int_opts:
             if opt in global_conf:
                 global_conf[opt] = conf.getint("Connection", opt)
-        # Boolean options
 
+        # Boolean options
         for opt in bool_opts:
             if opt in global_conf:
                 global_conf[opt] = conf.getboolean("Connection", opt)
+
+        # Other options, defer parsing to configuration.config_to_yaml:
+        connection_config = config_to_yaml(conf)["Connection"]
+        for opt in other_opts:
+            if opt in global_conf:
+                global_conf[opt] = connection_config[opt]
+
     local_conf = _make_conf(_DEFAULT_CONF, global_conf, kwargs)
 
     connection = _Connection.get_instance()

[I. Nüske]

The timeout option cannot be retrieved as an int anymore, so it needs to be extracted in a new option category. We reuse the parsing already implemented in config_to_yaml.

int_opts is now empty, but I did not remove it as we might introduce new options that are integers. Please recheck if this should be removed anyways and re-implemented if needed.

Additionally, I was wondering whether there are any reasons not to also defer the parsing of int_opts and bool_opts to config_to_yaml - this would have the advantage that all parsing would be done in one method, making future changes easier and the code here simpler. If this would make sense, I can add it to this MR.

[Alexander Schlemmer]

I agree. I also don't see the motivation why this was split up into two separate places. Maybe you can use git-blame to find the original author and ask, whether there is a good reason to keep it. :-)