From 4726e0faac417a68d90b471c92fa8f21b2130cb9 Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Tue, 6 Aug 2024 17:30:50 +0530 Subject: [PATCH 1/6] incoperate signals and contextvars to record staticfiles for concurrent requests --- debug_toolbar/panels/staticfiles.py | 52 +++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/debug_toolbar/panels/staticfiles.py b/debug_toolbar/panels/staticfiles.py index 061068a30..84d3e884a 100644 --- a/debug_toolbar/panels/staticfiles.py +++ b/debug_toolbar/panels/staticfiles.py @@ -1,9 +1,11 @@ import contextlib +import uuid from contextvars import ContextVar from os.path import join, normpath from django.conf import settings from django.contrib.staticfiles import finders, storage +from django.dispatch import Signal from django.utils.functional import LazyObject from django.utils.translation import gettext_lazy as _, ngettext @@ -29,7 +31,9 @@ def url(self): # This will collect the StaticFile instances across threads. -used_static_files = ContextVar("djdt_static_used_static_files") +used_static_files = ContextVar("djdt_static_used_static_files", default=[]) +request_id_context_var = ContextVar("djdt_request_id_store") +record_static_file_signal = Signal() class DebugConfiguredStorage(LazyObject): @@ -59,7 +63,12 @@ def url(self, path): # The ContextVar wasn't set yet. Since the toolbar wasn't properly # configured to handle this request, we don't need to capture # the static file. - used_static_files.get().append(StaticFile(path)) + request_id = request_id_context_var.get() + record_static_file_signal.send( + sender=self, + staticfile=StaticFile(path), + request_id=request_id, + ) return super().url(path) self._wrapped = DebugStaticFilesStorage() @@ -73,7 +82,7 @@ class StaticFilesPanel(panels.Panel): A panel to display the found staticfiles. """ - is_async = False + is_async = True name = "Static files" template = "debug_toolbar/panels/staticfiles.html" @@ -88,12 +97,25 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.num_found = 0 self.used_paths = [] + self.request_id = str(uuid.uuid4()) + + def _store_static_files_signal_handler(self, sender, staticfile, **kwargs): + with contextlib.suppress(LookupError): + # Only record the static file if the request_id matches the one + # that was used to create the panel. + # as sender of the signal and this handler will have multiple + # concurrent connections and we want to avoid storing of same + # staticfile from other connections as well. + if request_id_context_var.get() == self.request_id: + used_static_files.get().append(staticfile) def enable_instrumentation(self): storage.staticfiles_storage = DebugConfiguredStorage() + record_static_file_signal.connect(self._store_static_files_signal_handler) + request_id_context_var.set(self.request_id) def disable_instrumentation(self): - storage.staticfiles_storage = _original_storage + record_static_file_signal.disconnect(self._store_static_files_signal_handler) @property def num_used(self): @@ -109,18 +131,20 @@ def nav_subtitle(self): "%(num_used)s file used", "%(num_used)s files used", num_used ) % {"num_used": num_used} - def process_request(self, request): - reset_token = used_static_files.set([]) - response = super().process_request(request) - # Make a copy of the used paths so that when the - # ContextVar is reset, our panel still has the data. - self.used_paths = used_static_files.get().copy() - # Reset the ContextVar to be empty again, removing the reference - # to the list of used files. - used_static_files.reset(reset_token) - return response + # def process_request(self, request): + # reset_token = used_static_files.set([]) + # response = super().process_request(request) + # # Make a copy of the used paths so that when the + # # ContextVar is reset, our panel still has the data. + # self.used_paths = used_static_files.get().copy() + # # Reset the ContextVar to be empty again, removing the reference + # # to the list of used files. + # used_static_files.reset(reset_token) + # return response def generate_stats(self, request, response): + self.used_paths = used_static_files.get().copy() + used_static_files.get().clear() self.record_stats( { "num_found": self.num_found, From d5bdcc3962fdc304b6c1655a5bbb6a5d05460038 Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Thu, 8 Aug 2024 20:18:54 +0530 Subject: [PATCH 2/6] remove used_static_files contextvar and its dependencies allow on app ready monkey patching --- debug_toolbar/panels/staticfiles.py | 40 +++++++++++------------------ 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/debug_toolbar/panels/staticfiles.py b/debug_toolbar/panels/staticfiles.py index 84d3e884a..ea50968b6 100644 --- a/debug_toolbar/panels/staticfiles.py +++ b/debug_toolbar/panels/staticfiles.py @@ -30,8 +30,8 @@ def url(self): return storage.staticfiles_storage.url(self.path) -# This will collect the StaticFile instances across threads. -used_static_files = ContextVar("djdt_static_used_static_files", default=[]) +# This will record and map the StaticFile instances with its associated +# request across threads and async concurrent requests state. request_id_context_var = ContextVar("djdt_request_id_store") record_static_file_signal = Signal() @@ -99,23 +99,26 @@ def __init__(self, *args, **kwargs): self.used_paths = [] self.request_id = str(uuid.uuid4()) + @classmethod + def ready(cls): + storage.staticfiles_storage = DebugConfiguredStorage() + def _store_static_files_signal_handler(self, sender, staticfile, **kwargs): - with contextlib.suppress(LookupError): - # Only record the static file if the request_id matches the one - # that was used to create the panel. - # as sender of the signal and this handler will have multiple - # concurrent connections and we want to avoid storing of same - # staticfile from other connections as well. - if request_id_context_var.get() == self.request_id: - used_static_files.get().append(staticfile) + # Only record the static file if the request_id matches the one + # that was used to create the panel. + # as sender of the signal and this handler will have multiple + # concurrent connections and we want to avoid storing of same + # staticfile from other connections as well. + if request_id_context_var.get() == self.request_id: + self.used_paths.append(staticfile) def enable_instrumentation(self): - storage.staticfiles_storage = DebugConfiguredStorage() record_static_file_signal.connect(self._store_static_files_signal_handler) - request_id_context_var.set(self.request_id) + self.ctx_token = request_id_context_var.set(self.request_id) def disable_instrumentation(self): record_static_file_signal.disconnect(self._store_static_files_signal_handler) + request_id_context_var.reset(self.ctx_token) @property def num_used(self): @@ -131,20 +134,7 @@ def nav_subtitle(self): "%(num_used)s file used", "%(num_used)s files used", num_used ) % {"num_used": num_used} - # def process_request(self, request): - # reset_token = used_static_files.set([]) - # response = super().process_request(request) - # # Make a copy of the used paths so that when the - # # ContextVar is reset, our panel still has the data. - # self.used_paths = used_static_files.get().copy() - # # Reset the ContextVar to be empty again, removing the reference - # # to the list of used files. - # used_static_files.reset(reset_token) - # return response - def generate_stats(self, request, response): - self.used_paths = used_static_files.get().copy() - used_static_files.get().clear() self.record_stats( { "num_found": self.num_found, From 182e90bf60c3bad7aefe0b613058ad241313f319 Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Mon, 19 Aug 2024 20:55:02 +0530 Subject: [PATCH 3/6] async static files panel test --- tests/panels/test_staticfiles.py | 13 +++++++++++++ tests/templates/base.html | 1 + tests/templates/staticfiles/async_static.html | 6 ++++++ 3 files changed, 20 insertions(+) create mode 100644 tests/templates/staticfiles/async_static.html diff --git a/tests/panels/test_staticfiles.py b/tests/panels/test_staticfiles.py index 0736d86ed..11e89e1a2 100644 --- a/tests/panels/test_staticfiles.py +++ b/tests/panels/test_staticfiles.py @@ -1,5 +1,7 @@ from django.conf import settings from django.contrib.staticfiles import finders +from django.shortcuts import render +from django.test import AsyncRequestFactory from ..base import BaseTestCase @@ -27,6 +29,17 @@ def test_default_case(self): self.panel.get_staticfiles_dirs(), finders.FileSystemFinder().locations ) + async def test_store_staticfiles_with_async_context(self): + async def get_response(request): + # template contains one static file + return render(request, "staticfiles/async_static.html") + + self._get_response = get_response + async_request = AsyncRequestFactory().get("/") + response = await self.panel.process_request(async_request) + self.panel.generate_stats(self.request, response) + self.assertNotEqual(self.panel.num_used, 0) + def test_insert_content(self): """ Test that the panel only inserts content after generate_stats and diff --git a/tests/templates/base.html b/tests/templates/base.html index ea0d773ac..272c316f0 100644 --- a/tests/templates/base.html +++ b/tests/templates/base.html @@ -2,6 +2,7 @@ + {% block head %}{% endblock %} {% block content %}{% endblock %} diff --git a/tests/templates/staticfiles/async_static.html b/tests/templates/staticfiles/async_static.html new file mode 100644 index 000000000..fc0c9b885 --- /dev/null +++ b/tests/templates/staticfiles/async_static.html @@ -0,0 +1,6 @@ +{% extends "base.html" %} +{% load static %} + +{% block head %} + +{% endblock %} From 0c3dbe58d92222e69c1e0273918e645e9cb45c78 Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Mon, 19 Aug 2024 20:56:39 +0530 Subject: [PATCH 4/6] update doc --- docs/architecture.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture.rst b/docs/architecture.rst index 145676459..e30aabbe8 100644 --- a/docs/architecture.rst +++ b/docs/architecture.rst @@ -81,7 +81,7 @@ Problematic Parts the main benefit of the toolbar - Support for async and multi-threading: ``debug_toolbar.middleware.DebugToolbarMiddleware`` is now async compatible and can process async requests. However certain - panels such as ``SQLPanel``, ``TimerPanel``, ``StaticFilesPanel``, + panels such as ``SQLPanel``, ``TimerPanel``, ``RequestPanel``, ``RedirectsPanel`` and ``ProfilingPanel`` aren't fully compatible and currently being worked on. For now, these panels are disabled by default when running in async environment. From cf0d2a0c95dba39fa4cce9346619472ed67dc7ee Mon Sep 17 00:00:00 2001 From: Tim Schilling Date: Tue, 20 Aug 2024 07:10:56 -0500 Subject: [PATCH 5/6] Code review changes --- debug_toolbar/panels/staticfiles.py | 2 +- tests/panels/test_staticfiles.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/debug_toolbar/panels/staticfiles.py b/debug_toolbar/panels/staticfiles.py index ea50968b6..b0997404c 100644 --- a/debug_toolbar/panels/staticfiles.py +++ b/debug_toolbar/panels/staticfiles.py @@ -113,8 +113,8 @@ def _store_static_files_signal_handler(self, sender, staticfile, **kwargs): self.used_paths.append(staticfile) def enable_instrumentation(self): - record_static_file_signal.connect(self._store_static_files_signal_handler) self.ctx_token = request_id_context_var.set(self.request_id) + record_static_file_signal.connect(self._store_static_files_signal_handler) def disable_instrumentation(self): record_static_file_signal.disconnect(self._store_static_files_signal_handler) diff --git a/tests/panels/test_staticfiles.py b/tests/panels/test_staticfiles.py index 11e89e1a2..3caedc4eb 100644 --- a/tests/panels/test_staticfiles.py +++ b/tests/panels/test_staticfiles.py @@ -38,7 +38,7 @@ async def get_response(request): async_request = AsyncRequestFactory().get("/") response = await self.panel.process_request(async_request) self.panel.generate_stats(self.request, response) - self.assertNotEqual(self.panel.num_used, 0) + self.assertEqual(self.panel.num_used, 1) def test_insert_content(self): """ From ed40ead5dc1d5083d16a1960a6ec6e0f5819e4c2 Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Tue, 20 Aug 2024 17:55:18 +0530 Subject: [PATCH 6/6] suggested changes --- debug_toolbar/panels/staticfiles.py | 2 +- tests/panels/test_staticfiles.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/debug_toolbar/panels/staticfiles.py b/debug_toolbar/panels/staticfiles.py index ea50968b6..b0997404c 100644 --- a/debug_toolbar/panels/staticfiles.py +++ b/debug_toolbar/panels/staticfiles.py @@ -113,8 +113,8 @@ def _store_static_files_signal_handler(self, sender, staticfile, **kwargs): self.used_paths.append(staticfile) def enable_instrumentation(self): - record_static_file_signal.connect(self._store_static_files_signal_handler) self.ctx_token = request_id_context_var.set(self.request_id) + record_static_file_signal.connect(self._store_static_files_signal_handler) def disable_instrumentation(self): record_static_file_signal.disconnect(self._store_static_files_signal_handler) diff --git a/tests/panels/test_staticfiles.py b/tests/panels/test_staticfiles.py index 11e89e1a2..3caedc4eb 100644 --- a/tests/panels/test_staticfiles.py +++ b/tests/panels/test_staticfiles.py @@ -38,7 +38,7 @@ async def get_response(request): async_request = AsyncRequestFactory().get("/") response = await self.panel.process_request(async_request) self.panel.generate_stats(self.request, response) - self.assertNotEqual(self.panel.num_used, 0) + self.assertEqual(self.panel.num_used, 1) def test_insert_content(self): """