Merge pull request #6028 from PrimozGodec/change-proxy-behaviour

[FIX] Embedders - fix proxies, default on http, tests

Author: lanzagar

Orange/misc/tests/test_embedder_utils.py

@@ -0,0 +1,67 @@
+import os
+import unittest
+
+from Orange.misc.utils.embedder_utils import get_proxies
+
+
+class TestProxies(unittest.TestCase):
+    def setUp(self) -> None:
+        self.previous_http = os.environ.get("http_proxy")
+        self.previous_https = os.environ.get("https_proxy")
+        os.environ.pop("http_proxy", None)
+        os.environ.pop("https_proxy", None)
+
+    def tearDown(self) -> None:
+        os.environ.pop("http_proxy", None)
+        os.environ.pop("https_proxy", None)
+        if self.previous_http is not None:
+            os.environ["http_proxy"] = self.previous_http
+        if self.previous_https is not None:
+            os.environ["https_proxy"] = self.previous_https
+
+    def test_add_scheme(self):
+        os.environ["http_proxy"] = "test1.com"
+        os.environ["https_proxy"] = "test2.com"
+        res = get_proxies()
+        self.assertEqual("http://test1.com", res.get("http://"))
+        self.assertEqual("http://test2.com", res.get("https://"))
+
+        os.environ["http_proxy"] = "test1.com/path"
+        os.environ["https_proxy"] = "test2.com/path"
+        res = get_proxies()
+        self.assertEqual("http://test1.com/path", res.get("http://"))
+        self.assertEqual("http://test2.com/path", res.get("https://"))
+
+        os.environ["http_proxy"] = "https://test1.com:123"
+        os.environ["https_proxy"] = "https://test2.com:124"
+        res = get_proxies()
+        self.assertEqual("https://test1.com:123", res.get("http://"))
+        self.assertEqual("https://test2.com:124", res.get("https://"))
+
+    def test_both_urls(self):
+        os.environ["http_proxy"] = "http://test1.com:123"
+        os.environ["https_proxy"] = "https://test2.com:124"
+        res = get_proxies()
+        self.assertEqual("http://test1.com:123", res.get("http://"))
+        self.assertEqual("https://test2.com:124", res.get("https://"))
+        self.assertNotIn("all://", res)
+
+    def test_http_only(self):
+        os.environ["http_proxy"] = "http://test1.com:123"
+        res = get_proxies()
+        self.assertEqual("http://test1.com:123", res.get("http://"))
+        self.assertNotIn("https://", res)
+
+    def test_https_only(self):
+        os.environ["https_proxy"] = "https://test1.com:123"
+        res = get_proxies()
+        self.assertEqual("https://test1.com:123", res.get("https://"))
+        self.assertNotIn("http://", res)
+
+    def test_none(self):
+        """ When no variable is set return None """
+        self.assertIsNone(get_proxies())
+
+
+if __name__ == "__main__":
+    unittest.main()

Orange/misc/utils/embedder_utils.py

@@ -78,22 +78,26 @@ def add(self, cache_key, value):
 
 def get_proxies() -> Optional[Dict[str, str]]:
     """
-    Return dict with proxy addresses if they exists.
+    Return dict with proxy addresses if they exist.
 
     Returns
     -------
     proxy_dict
         Dictionary with format {proxy type: proxy address} or None if
         they not set.
     """
-    def add_protocol(url: Optional[str], prot: str) -> Optional[str]:
-        if url and not url.startswith(prot):
-            return f"{prot}://{url}"
-        return url
-    http_proxy = add_protocol(environ.get("http_proxy"), "http")
-    https_proxy = add_protocol(environ.get("https_proxy"), "https")
-    if http_proxy and https_proxy:  # both proxy addresses defined
-        return {"http://": https_proxy, "https://": https_proxy}
-    elif any([https_proxy, http_proxy]):  # one of the proxies defined
-        return {"all://": http_proxy or https_proxy}
-    return None  # proxies not defined
+    def add_scheme(url: Optional[str]) -> Optional[str]:
+        if url is not None and "://" not in url:
+            # if no scheme default to http - as other libraries do (e.g. requests)
+            return f"http://{url}"
+        else:
+            return url
+
+    http_proxy = add_scheme(environ.get("http_proxy"))
+    https_proxy = add_scheme(environ.get("https_proxy"))
+    proxy_dict = {}
+    if http_proxy:
+        proxy_dict["http://"] = http_proxy
+    if https_proxy:
+        proxy_dict["https://"] = https_proxy
+    return proxy_dict if proxy_dict else None