refactor: clean up some pylint warnings

Author: apotterri

appmap/_implementation/django.py

@@ -12,7 +12,7 @@
 from . import remote_recording
 
 
-class RemoteRecording:
+class RemoteRecording:  # pylint: disable=missing-class-docstring
     def __init__(self, get_response):
         super().__init__()
         self.get_response = get_response

appmap/_implementation/event.py

@@ -1,3 +1,4 @@
+# pylint: disable=missing-module-docstring,missing-class-docstring,missing-function-docstring
 import inspect
 import logging
 import threading

appmap/_implementation/flask.py

@@ -14,17 +14,17 @@
 
 @app.route("/record", methods=["GET"])
 def status():
-    body, status = remote_recording.status()
-    return Response(body, status=status, mimetype="application/json")
+    body, rrstatus = remote_recording.status()
+    return Response(body, status=rrstatus, mimetype="application/json")
 
 
 @app.route("/record", methods=["POST"])
 def start():
-    body, status = remote_recording.start()
-    return Response(body, status=status, mimetype="application/json")
+    body, rrstatus = remote_recording.start()
+    return Response(body, status=rrstatus, mimetype="application/json")
 
 
 @app.route("/record", methods=["DELETE"])
 def stop():
-    body, status = remote_recording.stop()
-    return Response(body, status=status, mimetype="application/json")
+    body, rrstatus = remote_recording.stop()
+    return Response(body, status=rrstatus, mimetype="application/json")

appmap/_implementation/recorder.py

@@ -2,12 +2,12 @@
 import threading
 import traceback
 from abc import ABC, abstractmethod
-from copy import copy
 
 from appmap._implementation.utils import appmap_tls
 
 logger = logging.getLogger(__name__)
 
+# pylint: disable=global-statement
 _default_recorder = None
 
 
@@ -70,19 +70,19 @@ def new_global(cls):
 
     @classmethod
     def get_enabled(cls):
-        return cls.get_current()._enabled
+        return cls.get_current()._enabled  # pylint: disable=protected-access
 
     @classmethod
     def set_enabled(cls, e):
-        cls.get_current()._enabled = e
+        cls.get_current()._enabled = e  # pylint: disable=protected-access
 
     @classmethod
     def start_recording(cls):
-        cls.get_current()._start_recording()
+        cls.get_current()._start_recording()  # pylint: disable=protected-access
 
     @classmethod
     def stop_recording(cls):
-        return cls.get_current()._stop_recording()
+        return cls.get_current()._stop_recording()  # pylint: disable=protected-access
 
     @classmethod
     def add_event(cls, event):
@@ -91,15 +91,14 @@ def add_event(cls, event):
         one).
         """
         perthread, shared = cls._get_current()
-        shared._add_event(event)
+        shared._add_event(event)  # pylint: disable=protected-access
         if perthread:
-            perthread._add_event(event)
+            perthread._add_event(event)  # pylint: disable=protected-access
 
     _RECORDER_KEY = "appmap_recorder"
 
     @classmethod
     def _get_current(cls):
-        global _default_recorder
         perthread = appmap_tls().get(cls._RECORDER_KEY, None)
 
         return [perthread, _default_recorder]
@@ -153,6 +152,8 @@ class ThreadRecorder(Recorder):
     def events(self):
         return super().events
 
+    # They're not useless, because they're abtract with a default implementation
+    # pragma pylint: disable=useless-super-delegation
     def _start_recording(self):
         super()._start_recording()
 
@@ -162,6 +163,8 @@ def _stop_recording(self):
     def _add_event(self, event):
         super()._add_event(event)
 
+    # pragma pylint: enable=useless-super-delegation
+
 
 class SharedRecorder(Recorder):
     """
@@ -197,4 +200,4 @@ def _add_event(self, event):
 
 
 def initialize():
-    Recorder._initialize()
+    Recorder._initialize()  # pylint: disable=protected-access

appmap/_implementation/remote_recording.py

@@ -6,6 +6,7 @@
 from .detect_enabled import DetectEnabled
 from .recorder import Recorder
 
+# pylint: disable=global-statement
 _enabled_lock = Lock()
 _enabled = False
 

appmap/_implementation/web_framework.py

@@ -13,7 +13,7 @@
 from . import generation, remote_recording
 from .detect_enabled import DetectEnabled
 from .env import Env
-from .event import Event, ReturnEvent, _EventIds, describe_value
+from .event import Event, ReturnEvent, describe_value
 from .recorder import Recorder, ThreadRecorder
 from .utils import root_relative_path, scenario_filename
 

appmap/django.py

@@ -16,15 +16,13 @@
 from django.db.backends.signals import connection_created
 from django.db.backends.utils import CursorDebugWrapper
 from django.dispatch import receiver
-from django.http import HttpResponse, HttpResponseBadRequest, JsonResponse
 from django.template import Template
 from django.urls import get_resolver, resolve
 from django.urls.exceptions import Resolver404
 from django.urls.resolvers import _route_to_regex
 
 from appmap._implementation.detect_enabled import DetectEnabled
 
-from ._implementation import generation
 from ._implementation.event import (
     ExceptionEvent,
     HttpServerRequestEvent,

appmap/flask.py

@@ -1,12 +1,10 @@
-import pdb
 import re
-import sys
 import time
 
 import flask
 import flask.cli
 import jinja2
-from flask import _app_ctx_stack, current_app, request
+from flask import g, request
 from flask.cli import ScriptInfo
 from werkzeug.exceptions import BadRequest
 from werkzeug.middleware.dispatcher import DispatcherMiddleware
@@ -16,7 +14,6 @@
 from appmap._implementation.env import Env
 from appmap._implementation.event import HttpServerRequestEvent, HttpServerResponseEvent
 from appmap._implementation.flask import app as remote_recording_app
-from appmap._implementation.recorder import Recorder
 from appmap._implementation.web_framework import AppmapMiddleware
 from appmap._implementation.web_framework import TemplateHandler as BaseTemplateHandler
 
@@ -65,9 +62,6 @@ class AppmapFlask(AppmapMiddleware):
     ```
     """
 
-    def __init__(self):
-        super().__init__()
-
     def init_app(self, app):
         if DetectEnabled.should_enable("remote"):
             app.wsgi_app = DispatcherMiddleware(
@@ -87,13 +81,15 @@ def before_request_main(self, rec, request):
         Metadata.add_framework("flask", flask.__version__)
         np = None
         if request.url_rule:
+            # pragma pylint: disable=line-too-long
             # Transform request.url to the expected normalized-path form. For example,
             # "/post/<username>/<post_id>/summary" becomes "/post/{username}/{post_id}/summary".
             # Notes:
             #   * the value of `repr` of this rule begins with "<Rule '/post/<username>/<post_id>/summary'"
             #   * the variable names in a rule can only contain alphanumerics:
             #     * flask 1: https://github.com/pallets/werkzeug/blob/1dde4b1790f9c46b7122bb8225e6b48a5b22a615/src/werkzeug/routing.py#L143
             #     * flask 2: https://github.com/pallets/werkzeug/blob/99f328cf2721e913bd8a3128a9cdd95ca97c334c/src/werkzeug/routing/rules.py#L56
+            # pragma pylint: enable=line-too-long
             r = repr(request.url_rule)
             np = NP_PARAMS.findall(r)[0].translate(NP_PARAM_DELIMS)
 
@@ -107,10 +103,10 @@ def before_request_main(self, rec, request):
         )
         rec.add_event(call_event)
 
-        appctx = _app_ctx_stack.top
-        appctx.appmap_request_event = call_event
-        appctx.appmap_request_start = time.monotonic()
-
+        # Flask 2 removed the suggestion to use _app_ctx_stack.top, and instead says extensions
+        # should use g with a private property.
+        g._appmap_request_event = call_event  # pylint: disable=protected-access
+        g._appmap_request_start = time.monotonic()  # pylint: disable=protected-access
         return None, None
 
     def after_request(self, response):
@@ -128,10 +124,9 @@ def after_request(self, response):
         )
 
     def after_request_main(self, rec, response, start, call_event_id):
-        appctx = _app_ctx_stack.top
-        parent_id = appctx.appmap_request_event.id
-        duration = time.monotonic() - appctx.appmap_request_start
-
+        parent_id = g._appmap_request_event.id  # pylint: disable=protected-access
+        start = g._appmap_request_start  # pylint: disable=protected-access
+        duration = time.monotonic() - start
         return_event = HttpServerResponseEvent(
             parent_id=parent_id,
             elapsed=duration,

appmap/test/test_django.py

@@ -1,9 +1,10 @@
 # flake8: noqa: E402
-# pylint: disable=unused-import, redefined-outer-name, missing-function-docstring
+# pylint: disable=missing-function-docstring,redefined-outer-name
 
 import json
 import sys
 from pathlib import Path
+from threading import Thread
 
 import django
 import django.conf
@@ -19,23 +20,17 @@
 import appmap.django  # noqa: F401
 from appmap.test.helpers import DictIncluding
 
+from .._implementation.metadata import Metadata
+from .web_framework import TestRecordRequests, exec_cmd, wait_until_port_is
+
 sys.path += [str(Path(__file__).parent / "data" / "django")]
-import app
 
-from .._implementation.metadata import Metadata
+# Import app just for the side-effects. It must happen after sys.path has been modified.
+import app  # pyright: ignore pylint: disable=import-error, unused-import,wrong-import-order
 
 # Make sure assertions in web_framework get rewritten (e.g. to show
 # diffs in generated appmaps)
 pytest.register_assert_rewrite("appmap.test.web_framework")
-from threading import Thread
-
-from .web_framework import (
-    TestRecording,
-    TestRecordRequests,
-    TestRequestCapture,
-    exec_cmd,
-    wait_until_port_is,
-)
 
 
 @pytest.mark.django_db
@@ -235,7 +230,11 @@ def server_start(env_vars_str):
     @staticmethod
     def server_stop():
         exec_cmd(
-            "ps -ef | grep -i 'manage.py runserver' | grep -v grep | awk '{ print $2 }' | xargs kill -9"
+            "ps -ef"
+            + "| grep -i 'manage.py runserver'"
+            + "| grep -v grep"
+            + "| awk '{ print $2 }'"
+            + "| xargs kill -9"
         )
         wait_until_port_is("127.0.0.1", TestRecordRequests.server_port, "closed")
 

appmap/test/test_flask.py

@@ -12,19 +12,16 @@
 from appmap.test.helpers import DictIncluding
 
 from .._implementation.metadata import Metadata
-
-# Make sure assertions in web_framework get rewritten (e.g. to show
-# diffs in generated appmaps)
-pytest.register_assert_rewrite("appmap.test.web_framework")
-
 from .web_framework import (  # pylint: disable=unused-import
-    TestRecording,
     TestRecordRequests,
-    TestRequestCapture,
     exec_cmd,
     wait_until_port_is,
 )
 
+# Make sure assertions in web_framework get rewritten (e.g. to show
+# diffs in generated appmaps)
+pytest.register_assert_rewrite("appmap.test.web_framework")
+
 
 @pytest.fixture(name="client")
 def flask_client(app):
@@ -38,7 +35,7 @@ def flask_app(data_dir, monkeypatch):
 
     Env.current.set("APPMAP_CONFIG", data_dir / "flask" / "appmap.yml")
 
-    import app  # pylint: disable=import-error
+    import app  # pyright: ignore pylint: disable=import-error,import-outside-toplevel
 
     importlib.reload(app)