Repository: scoutapp/scout_apm_python
Pull request #480 (2020-02-17 14:37:34)

Move from requests to urllib3
Basically avoid using `requests` so that we aren't pulling it in for people who aren't using it. Whilst popular I think some projects will be moving to httpx and it's best to minimize our footprint. We only use it for the single download of the core agent.

adamchainz: Error is in a new flaky looking test:

```
=================================== FAILURES ===================================
___________________________ test_report_app_metadata ___________________________
send = <MagicMock name='send' id='140358827793152'>
    @mock.patch("scout_apm.core.socket.CoreAgentSocketThread.send")
    def test_report_app_metadata(send):
        report_app_metadata()


>       assert send.call_count == 1
E       AssertionError: assert 3 == 1
E        +  where 3 = <MagicMock name='send' id='140358827793152'>.call_count
tests/unit/core/test_metadata.py:17: AssertionError
```

It's only happening on Python 3.5, randomly on one of the environments. Will investigate.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1799677..fd0f653 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,12 @@
   This ensures that all traces are recorded, rather than lost, especially
   useful for single-run background commands.
   ([Issue # 473](https://github.com/scoutapp/scout_apm_python/issues/473)).
+- Move from **requests** to underlying **urllib3** for downloads to reduce
+  dependency footprint
+  ([PR #480](https://github.com/scoutapp/scout_apm_python/pull/480)).
+- Retry downloading the core agent a few times, and use a timeout to prevent
+  startup hangs
+  ([PR #480](https://github.com/scoutapp/scout_apm_python/pull/480)).
 
 ### Fixed
 
diff --git a/setup.py b/setup.py
index df5892c..d6eeaf8 100644
--- a/setup.py
+++ b/setup.py
@@ -59,7 +59,8 @@ setup(
         'asgiref ; python_version >= "3.5"',
         'importlib-metadata ; python_version < "3.8"',
         "psutil>=5,<6",
-        "requests>=2,<3",
+        'urllib3[secure] < 1.25 ; python_version < "3.5"',
+        'urllib3[secure] < 2 ; python_version >= "3.5"',
         "wrapt>=1.10,<2.0",
     ],
     keywords="apm performance monitoring development",
diff --git a/src/scout_apm/compat.py b/src/scout_apm/compat.py
index ad7a83d..6327c12 100644
--- a/src/scout_apm/compat.py
+++ b/src/scout_apm/compat.py
@@ -6,6 +6,9 @@ import inspect
 import sys
 from functools import wraps
 
+import certifi
+import urllib3
+
 string_type = str if sys.version_info[0] >= 3 else basestring  # noqa: F821
 text_type = str if sys.version_info[0] >= 3 else unicode  # noqa: F821
 string_types = tuple({string_type, text_type})
@@ -119,6 +122,14 @@ def kwargs_only(func):
     return wrapper
 
 
+def urllib3_cert_pool_manager(**kwargs):
+    if sys.version_info >= (3, 0):
+        CERT_REQUIRED = "CERT_REQUIRED"
+    else:
+        CERT_REQUIRED = b"CERT_REQUIRED"
+    return urllib3.PoolManager(cert_reqs=CERT_REQUIRED, ca_certs=certifi.where())
+
+
 __all__ = [
     "ContextDecorator",
     "datetime_to_timestamp",
diff --git a/src/scout_apm/core/core_agent_manager.py b/src/scout_apm/core/core_agent_manager.py
index f213906..df0eb50 100644
--- a/src/scout_apm/core/core_agent_manager.py
+++ b/src/scout_apm/core/core_agent_manager.py
@@ -10,8 +10,9 @@ import tarfile
 import time
 import warnings
 
-import requests
+from urllib3.exceptions import HTTPError
 
+from scout_apm.compat import urllib3_cert_pool_manager
 from scout_apm.core.config import scout_config
 
 logger = logging.getLogger(__name__)
@@ -152,9 +153,10 @@ class CoreAgentDownloader(object):
         self.obtain_download_lock()
         if self.download_lock_fd is not None:
             try:
-                self.download_package()
-                self.untar()
-            except OSError:
+                downloaded = self.download_package()
+                if downloaded:
+                    self.untar()
+            except (OSError, HTTPError):
                 logger.exception("Exception raised while downloading Core Agent")
             finally:
                 self.release_download_lock()
@@ -195,11 +197,21 @@ class CoreAgentDownloader(object):
             os.close(self.download_lock_fd)
 
     def download_package(self):
-        logger.debug("Downloading: %s to %s", self.full_url(), self.package_location)
-        req = requests.get(self.full_url(), stream=True)
-        with open(self.package_location, "wb") as f:
-            for chunk in req.iter_content(1024 * 1000):
-                f.write(chunk)
+        full_url = self.full_url()
+        logger.debug("Downloading: %s to %s", full_url, self.package_location)
+        http = urllib3_cert_pool_manager()
+        response = http.request(
+            "GET", full_url, preload_content=False, timeout=10.0, retries=3
+        )
+        try:
+            if response.status != 200:
+                return False
+            with open(self.package_location, "wb") as fp:
+                for chunk in response.stream():
+                    fp.write(chunk)
+        finally:
+            response.release_conn()
+        return True
 
     def untar(self):
         t = tarfile.open(self.package_location, "r")
diff --git a/tox.ini b/tox.ini
index 3774d5c..18dc158 100644
--- a/tox.ini
+++ b/tox.ini
@@ -70,7 +70,6 @@ deps =
     six
     sqlalchemy
     starlette ; python_version >= "3.6"
-    urllib3[secure]
     webtest
 commands =
     pytest {posargs}


Test patch:
diff --git a/pytest.ini b/pytest.ini
index c949d22..059443e 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -52,3 +52,5 @@ filterwarnings =
     ; Appears in both gettext and Django:
     ignore:parameter codeset is deprecated:DeprecationWarning
     ignore:"errors" is deprecated. Use "encoding_errors" instead:DeprecationWarning:redis.client
+    ; Triggered on Python 2.7 only:
+    ignore:unicode for buf is no longer accepted, use bytes:DeprecationWarning:urllib3.contrib.pyopenssl
diff --git a/tests/integration/instruments/test_urllib3.py b/tests/integration/instruments/test_urllib3.py
index 8a07e53..8e76582 100644
--- a/tests/integration/instruments/test_urllib3.py
+++ b/tests/integration/instruments/test_urllib3.py
@@ -2,13 +2,12 @@
 from __future__ import absolute_import, division, print_function, unicode_literals
 
 import logging
-import sys
 
-import certifi
 import httpretty
 import pytest
 import urllib3
 
+from scout_apm.compat import urllib3_cert_pool_manager
 from scout_apm.instruments.urllib3 import ensure_installed
 from tests.compat import mock
 from tests.tools import delete_attributes
@@ -17,11 +16,6 @@ mock_not_attempted = mock.patch(
     "scout_apm.instruments.urllib3.have_patched_pool_urlopen", new=False
 )
 
-if sys.version_info >= (3, 0):
-    CERT_REQUIRED = "CERT_REQUIRED"
-else:
-    CERT_REQUIRED = b"CERT_REQUIRED"
-
 
 def test_ensure_installed_twice(caplog):
     ensure_installed()
@@ -86,7 +80,7 @@ def test_request(tracked_request):
             httpretty.GET, "https://example.com/", body="Hello World!"
         )
 
-        http = urllib3.PoolManager(cert_reqs=CERT_REQUIRED, ca_certs=certifi.where())
+        http = urllib3_cert_pool_manager()
         response = http.request("GET", "https://example.com")
 
     assert response.status == 200
@@ -100,7 +94,7 @@ def test_request(tracked_request):
 def test_request_type_error(tracked_request):
     ensure_installed()
     with pytest.raises(TypeError):
-        http = urllib3.PoolManager(cert_reqs=CERT_REQUIRED, ca_certs=certifi.where())
+        http = urllib3_cert_pool_manager()
         connection = http.connection_from_host("example.com", scheme="https")
         connection.urlopen()
 
@@ -118,7 +112,7 @@ def test_request_no_absolute_url(caplog, tracked_request):
             httpretty.GET, "https://example.com/", body="Hello World!"
         )
 
-        http = urllib3.PoolManager(cert_reqs=CERT_REQUIRED, ca_certs=certifi.where())
+        http = urllib3_cert_pool_manager()
         response = http.request("GET", "https://example.com")
 
     assert response.status == 200