Repository: scoutapp/scout_apm_python
Pull request #177 (2019-05-11 10:12:28)

Move some unnecessary classes to functions
From Aymeric's suggestions. Each of these classes only had one function, so they are better off as single functions in Python.


Patch:
diff --git a/src/scout_apm/core/core_agent_manager.py b/src/scout_apm/core/core_agent_manager.py
index 3f33c7b..98b3464 100644
--- a/src/scout_apm/core/core_agent_manager.py
+++ b/src/scout_apm/core/core_agent_manager.py
@@ -111,7 +111,7 @@ class CoreAgentManager(object):
             return False
 
         bin_path = self.core_agent_dir + "/" + manifest.bin_name
-        if SHA256.digest(bin_path) == manifest.sha256:
+        if sha256_digest(bin_path) == manifest.sha256:
             self.core_agent_bin_path = bin_path
             self.core_agent_bin_version = manifest.bin_version
             return True
@@ -228,15 +228,13 @@ class CoreAgentManifest(object):
         return self.valid
 
 
-class SHA256(object):
-    @staticmethod
-    def digest(filename, block_size=65536):
-        try:
-            sha256 = hashlib.sha256()
-            with open(filename, "rb") as f:
-                for block in iter(lambda: f.read(block_size), b""):
-                    sha256.update(block)
-            return sha256.hexdigest()
-        except OSError as e:
-            logger.debug("Error on digest: %r", e)
-            return None
+def sha256_digest(filename, block_size=65536):
+    try:
+        sha256 = hashlib.sha256()
+        with open(filename, "rb") as f:
+            for block in iter(lambda: f.read(block_size), b""):
+                sha256.update(block)
+        return sha256.hexdigest()
+    except OSError as e:
+        logger.debug("Error on digest: %r", e)
+        return None
diff --git a/src/scout_apm/django/apps.py b/src/scout_apm/django/apps.py
index 8d65860..4bb3f55 100644
--- a/src/scout_apm/django/apps.py
+++ b/src/scout_apm/django/apps.py
@@ -4,11 +4,12 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 import logging
 
 from django.apps import AppConfig
+from django.conf import settings
 
 import scout_apm.core
-from scout_apm.django.config import ConfigAdapter
-from scout_apm.django.instruments.sql import SQLInstrument
-from scout_apm.django.instruments.template import TemplateInstrument
+from scout_apm.core.config import ScoutConfig
+from scout_apm.django.instruments.sql import install_sql_instrumentation
+from scout_apm.django.instruments.template import install_template_instrumentation
 
 logger = logging.getLogger(__name__)
 
@@ -18,8 +19,7 @@ class ScoutApmDjangoConfig(AppConfig):
     verbose_name = "Scout Apm (Django)"
 
     def ready(self):
-        # Copy django configuration to scout_apm's config
-        ConfigAdapter.install()
+        self.copy_config()
 
         # Finish installing the agent. If the agent isn't installed for any
         # reason, return without installing instruments
@@ -30,8 +30,19 @@ class ScoutApmDjangoConfig(AppConfig):
         self.install_middleware()
 
         # Setup Instruments
-        SQLInstrument.install()
-        TemplateInstrument.install()
+        install_sql_instrumentation()
+        install_template_instrumentation()
+
+    def copy_config(self):
+        configs = {}
+        if getattr(settings, "BASE_DIR", None) is not None:
+            configs["application_root"] = settings.BASE_DIR
+        for name in dir(settings):
+            if name.startswith("SCOUT_"):
+                value = getattr(settings, name)
+                clean_name = name.replace("SCOUT_", "").lower()
+                configs[clean_name] = value
+        ScoutConfig.set(**configs)
 
     def install_middleware(self):
         """
diff --git a/src/scout_apm/django/config.py b/src/scout_apm/django/config.py
deleted file mode 100644
index 6365adb..0000000
--- a/src/scout_apm/django/config.py
+++ /dev/null
@@ -1,24 +0,0 @@
-# coding=utf-8
-from __future__ import absolute_import, division, print_function, unicode_literals
-
-import logging
-
-from django.conf import settings
-
-from scout_apm.core.config import ScoutConfig
-
-logger = logging.getLogger(__name__)
-
-
-class ConfigAdapter(object):
-    @classmethod
-    def install(cls):
-        configs = {}
-        if getattr(settings, "BASE_DIR", None) is not None:
-            configs["application_root"] = settings.BASE_DIR
-        for name in dir(settings):
-            if name.startswith("SCOUT_"):
-                value = getattr(settings, name)
-                clean_name = name.replace("SCOUT_", "").lower()
-                configs[clean_name] = value
-        ScoutConfig.set(**configs)
diff --git a/src/scout_apm/django/instruments/sql.py b/src/scout_apm/django/instruments/sql.py
index e44395a..7ac3c75 100644
--- a/src/scout_apm/django/instruments/sql.py
+++ b/src/scout_apm/django/instruments/sql.py
@@ -49,23 +49,14 @@ class _DetailedTracingCursorWrapper(CursorWrapper):
                 span.capture_backtrace()
 
 
-class SQLInstrument(object):
-    installed = False
+def install_sql_instrumentation():
+    if getattr(install_sql_instrumentation, "installed", False):
+        return
+    install_sql_instrumentation.installed = True
 
-    @staticmethod
-    def install():
-        """
-        Installs ScoutApm SQL Instrumentation by monkeypatching the `cursor`
-        method of BaseDatabaseWrapper, to return a wrapper that instruments any
-        calls going through it.
-        """
-        if SQLInstrument.installed:
-            return
-        SQLInstrument.installed = True
+    @monkeypatch_method(BaseDatabaseWrapper)
+    def cursor(original, self, *args, **kwargs):
+        result = original(*args, **kwargs)
+        return _DetailedTracingCursorWrapper(result, self)
 
-        @monkeypatch_method(BaseDatabaseWrapper)
-        def cursor(original, self, *args, **kwargs):
-            result = original(*args, **kwargs)
-            return _DetailedTracingCursorWrapper(result, self)
-
-        logger.debug("Monkey patched SQL")
+    logger.debug("Monkey patched SQL")
diff --git a/src/scout_apm/django/instruments/template.py b/src/scout_apm/django/instruments/template.py
index 32367e0..3538751 100644
--- a/src/scout_apm/django/instruments/template.py
+++ b/src/scout_apm/django/instruments/template.py
@@ -47,55 +47,52 @@ def load(parser, token):  # pragma: no cover
     return defaulttags.load(decorating_parser, token)
 
 
-class TemplateInstrument(object):
-    installed = False
-
-    @staticmethod
-    def install():
-        # Our eventual aim is to patch the render() method on the Node objects
-        # corresponding to custom template tags. However we have to jump through
-        # a number of hoops in order to get access to the object.
-        #
-        # 1. Add ourselves to the set of built in template tags
-        #    This allows us to replace the 'load' template tag which controls
-        #    the loading of custom template tags
-        # 2. Delegate to default load with replaced parser
-        #    We provide our own parser class so we can catch and intercept
-        #    calls to add_library.
-        # 3. add_library receives a library of template tags
-        #    It iterates through each template tag, wrapping its compile function
-        # 4. compile is called as part of compiling the template
-        #    Our wrapper is called instead of the original templatetag compile
-        #    function. We delegate to the original function, but then modify
-        #    the resulting object by wrapping its render() function. This
-        #    render() function is what ends up being timed and appearing in the
-        #    tree.
-
-        # XXX: Stopped working in Django 1.9
-        #  add_to_builtins('apm.instruments.view')
-        if TemplateInstrument.installed:
-            return
-        TemplateInstrument.installed = True
-
-        @trace_method(Template)
-        def __init__(self, *args, **kwargs):
-            name = args[2] if len(args) >= 3 else "<Unknown Template>"
-            return ("Template/Compile", {"name": name})
-
-        @trace_method(Template)
-        def render(self, *args, **kwargs):
-            name = self.name if self.name is not None else "<Unknown Template>"
-            return ("Template/Render", {"name": name})
-
-        @trace_method(BlockNode)  # noqa: F811
-        def render(self, *args, **kwargs):
-            return ("Block/Render", {"name": self.name})
-
-        logger.debug("Monkey patched Templates")
-
-        # XXX: Figure this out, causes exception that the "resolve_context" key
-        # isn't in dict
-        # Also will need to figure out the name hash
-        #  @trace_method(TemplateResponse)
-        #  def resolve_context(self, *args, **kwargs):
-        #  return ('Template/Context', {})
+def install_template_instrumentation():
+    if getattr(install_template_instrumentation, "installed", False):
+        return
+    install_template_instrumentation.installed = True
+
+    # Our eventual aim is to patch the render() method on the Node objects
+    # corresponding to custom template tags. However we have to jump through
+    # a number of hoops in order to get access to the object.
+    #
+    # 1. Add ourselves to the set of built in template tags
+    #    This allows us to replace the 'load' template tag which controls
+    #    the loading of custom template tags
+    # 2. Delegate to default load with replaced parser
+    #    We provide our own parser class so we can catch and intercept
+    #    calls to add_library.
+    # 3. add_library receives a library of template tags
+    #    It iterates through each template tag, wrapping its compile function
+    # 4. compile is called as part of compiling the template
+    #    Our wrapper is called instead of the original templatetag compile
+    #    function. We delegate to the original function, but then modify
+    #    the resulting object by wrapping its render() function. This
+    #    render() function is what ends up being timed and appearing in the
+    #    tree.
+
+    # XXX: Stopped working in Django 1.9
+    #  add_to_builtins('apm.instruments.view')
+
+    @trace_method(Template)
+    def __init__(self, *args, **kwargs):
+        name = args[2] if len(args) >= 3 else "<Unknown Template>"
+        return ("Template/Compile", {"name": name})
+
+    @trace_method(Template)
+    def render(self, *args, **kwargs):
+        name = self.name if self.name is not None else "<Unknown Template>"
+        return ("Template/Render", {"name": name})
+
+    @trace_method(BlockNode)  # noqa: F811
+    def render(self, *args, **kwargs):
+        return ("Block/Render", {"name": self.name})
+
+    logger.debug("Monkey patched Templates")
+
+    # XXX: Figure this out, causes exception that the "resolve_context" key
+    # isn't in dict
+    # Also will need to figure out the name hash
+    #  @trace_method(TemplateResponse)
+    #  def resolve_context(self, *args, **kwargs):
+    #  return ('Template/Context', {})


Test patch:
diff --git a/tests/integration/core/test_core_agent_manager.py b/tests/integration/core/test_core_agent_manager.py
index 86178d3..4d44e75 100644
--- a/tests/integration/core/test_core_agent_manager.py
+++ b/tests/integration/core/test_core_agent_manager.py
@@ -91,7 +91,7 @@ def test_download_and_launch(core_agent_manager):
 @patch("scout_apm.core.core_agent_manager.logger")
 def test_verify_error(logger, core_agent_manager):
     with patch(
-        "scout_apm.core.core_agent_manager.SHA256.digest",
+        "scout_apm.core.core_agent_manager.sha256_digest",
         return_value="not the expected digest",
     ):
         # Patch out the download() method to avoid downloading again