django-debug-toolbar

django-debug-toolbar copied to clipboard

by upgrading django to version 4.0.2 view function on django-debug-toolbar doesn't display properly. The expected string should be the class name: DashboardView not "backend_base.views.view". "backend_base" is app name.

The class itself is very simple: class DashboardView(LoginRequiredMixin, NavBarMixin, TemplateView): template_name = 'dashboard.html'

backend_base.views.view

source of issue: mario-signorino on stackoverflow.

It would be interesting to see how you're instantiating the DashboardView... it seems that maybe debug_toolbar.utils.get_name_from_obj doesn't do the right thing.

The view is just as described:

class DashboardView(LoginRequiredMixin, TemplateView):
    template_name = 'dashboard.html' 

and it is linked in urls.py as:

path('', DashboardView.as_view(), name="dashboard"),

Everything is in this virtualenv with Python 3.10.4:

asgiref==3.5.2
async-timeout==4.0.2
beautifulsoup4==4.11.1
certifi==2022.6.15
charset-normalizer==2.0.12
Deprecated==1.2.13
Django==4.0.5
django-bootstrap5==21.3
django-debug-toolbar==3.4.0
greenlet==1.1.2
idna==3.3
jsons==1.6.3
mqtt==0.0.1
packaging==21.3
paho-mqtt==1.6.1
pika==1.2.1
psycopg2==2.9.3
pyparsing==3.0.9
redis==4.3.3
requests==2.28.0
soupsieve==2.3.2.post1
SQLAlchemy==1.4.37
sqlparse==0.4.2
typish==1.9.3
urllib3==1.26.9
wrapt==1.14.1

if I look in debug_toolbar.utils.get_name_from_obj I see:

    if hasattr(obj, "__name__"):
        name = obj.__name__
    else:
        name = obj.__class__.__name__

where (in my case) obj is: '<function View.as_view.<locals>.view at 0x7f26f3d5dea0>'.

The first if hasattr(obj, "__name__"): pass and set name to 'view'. Then the second if hasattr(obj, "__module__"): set module to 'backend_base.views' and hence, name to 'backend_base.views.view'`

The final status is:

Interesting. I see that the module path is correct. obj.__name__ didn't work for class-based views earlier so maybe something has changed in this area. We might try changing the code to prefer obj.__class__.__name__ to obj.__name__ if the former exists (even though I personally have a strong preference for FBVs over CBVs ;-)

Ah no, forget about that. We should do the same thing Django's admindocs app does, see

https://github.com/django/django/blob/9a22d1769b042a88741f0ff3087f10d94f325d86/django/contrib/admindocs/utils.py#L26-L32

yes, it seems to work: just replacing

def get_name_from_obj(obj):
    if hasattr(obj, "__name__"):
        name = obj.__name__
    else:
        name = obj.__class__.__name__

    if hasattr(obj, "__module__"):
        module = obj.__module__
        name = f"{module}.{name}"

    return name

with

def get_name_from_obj(view_func):
    if hasattr(view_func, "view_class"):
        klass = view_func.view_class
        return f"{klass.__module__}.{klass.__qualname__}"
    mod_name = view_func.__module__
    view_name = getattr(view_func, "__qualname__", view_func.__class__.__name__)
    return mod_name + "." + view_name

fix my problem. Not sure though about any side effects.

Do you want to submit this as a pull request? I think the code looks fine; if the testsuite passes then we're good.

I had ran a test with this change. And looks like all the tests related to this function fail:

FAIL: test_class (tests.test_utils.GetNameFromObjTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/leandro/dev/contrib/django-debug-toolbar/tests/test_utils.py", line 32, in test_class
    self.assertEqual(res, "tests.test_utils.A")
AssertionError: 'tests.test_utils.GetNameFromObjTestCase.test_class.<locals>.A' != 'tests.test_utils.A'
- tests.test_utils.GetNameFromObjTestCase.test_class.<locals>.A
+ tests.test_utils.A


======================================================================
FAIL: test_func (tests.test_utils.GetNameFromObjTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/leandro/dev/contrib/django-debug-toolbar/tests/test_utils.py", line 21, in test_func
    self.assertEqual(res, "tests.test_utils.x")
AssertionError: 'tests.test_utils.GetNameFromObjTestCase.test_func.<locals>.x' != 'tests.test_utils.x'
- tests.test_utils.GetNameFromObjTestCase.test_func.<locals>.x
+ tests.test_utils.x


======================================================================
FAIL: test_lambda (tests.test_utils.GetNameFromObjTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/leandro/dev/contrib/django-debug-toolbar/tests/test_utils.py", line 25, in test_lambda
    self.assertEqual(res, "tests.test_utils.<lambda>")
AssertionError: 'tests.test_utils.GetNameFromObjTestCase.test_lambda.<locals>.<lambda>' != 'tests.test_utils.<lambda>'
- tests.test_utils.GetNameFromObjTestCase.test_lambda.<locals>.<lambda>
+ tests.test_utils.<lambda>

Any update on this? It was a very useful feature..

Nobody took the time to submit a pull request until now, so there's no update yet, unfortunately :-/

@leandrodesouzadev are you working on this?

Actually no, i wasn't quite sure if i could just make the changes on the tests so they pass. Can i just make this changes?

I think that makes sense. Let's see it and decide.

I have implemented the feature and modified some tests to make it work. Now the code it's essentially the same as the quoted by @matthiask:

Ah no, forget about that. We should do the same thing Django's admindocs app does, see

https://github.com/django/django/blob/9a22d1769b042a88741f0ff3087f10d94f325d86/django/contrib/admindocs/utils.py#L26-L32

I wonder if there's any reason for us to keep this piece of code that's essentially the same as the django implementation. I think we have 3 options here:

1. Refactor our get_name_from_obj function to do the same as the django admindocs;
2. Refactor our get_name_from_obj function to be a proxy to the django admindocs get_view_name function;
3. Just use the get_view_name function directly from the django admindocs module.

For the first case, the implementation would be the following:

def get_name_from_obj(obj: Any) -> str:
    if hasattr(obj, "view_class"):
        klass = obj.view_class
        return f"{klass.__module__}.{klass.__qualname__}"
    mod_name = obj.__module__
    view_name = getattr(obj, "__qualname__", obj.__class__.__name__)
    return mod_name + "." + view_name

For the second case, the implementation would be the following:

from django.contrib.admindocs.utils import get_view_name
# ...
def get_name_from_obj(obj: Any) -> str:
    return get_view_name(obj)

For the third case, i saw that are we're going to need to change the following lines to use the get_view_name function from the django admindocs module: Panel class, on the __init__ method RequestPanel class, on the generate_stats method I see the benefit on this 3rd implementation that we could get away of the get_name_from_obj function, as well of the 3 tests for that utility function. But we have a drawback if they ever change this functionality.

Waiting on your suggestions. Cheers.

We could go the 3. route while keeping at least one unit test and it would probably be fine (we would be warned soon when the code is changed or removed in Django's main branch)

Certainly 1. raises less questions about depending on undocumented functions in Django. I think 1. is clearly the better option. Please add a comment where the code is from so that we can copy the code again if anything breaks in the future :-)