Update djdt-flamegraph to work with newest django debug toolbar - #9
Update djdt-flamegraph to work with newest django debug toolbar#9aleksanb wants to merge 1 commit into
Conversation
Lifecycle methods changed on djdt 2.0 release, and scripts stopped executing as well. This change requires you to add djdt_flamegraph to INSTALLED_APPS.
|
Hey @aleksanb, we don't really use this tool internally any more so I don't have a good way to test the changes. Happy to merge this though. However, why was the script extracted to its own file? Also, if extra steps are required to make this work, could you update the readme? Cheers. |
|
I installed this fork in my Django 2.2 project with django-debug-toolbar 2.2 and although the flamegraph tab now shows up in the toolbar, it is grayed out. My |
jayvdb
left a comment
jayvdb
left a comment
There was a problem hiding this comment.
This works for me, but appear to have more changes than necessary
| return Template(template).render(Context({ | ||
| 'flamegraph': flamegraph.stats_to_svg(self.sampler.get_stats()) | ||
| } | ||
| return Template(template).render(Context(ctx)) |
There was a problem hiding this comment.
This doesnt appear to have changed at all? Why modify it?
There was a problem hiding this comment.
It's a leftover from other attempts to fix the scripts not executing that i ended up removing.
I can remove it?
There was a problem hiding this comment.
Restoring this chunk would be good.
Also I think the minimum supported version of djdt should be increased to 2.2 - that is what works for me.
There was a problem hiding this comment.
ping @aleksanb , can you restore this, so that the changes needed are more clear and not distorted by unneeded changes.
Hi. A thing i see i forgot to mention here, is that we're running django debug toolbar from master, as when debugging why the script moving content to the iframe wasn't executing in the frontend, i found this notice in the changelogs for unreleased djdt 3.0: https://django-debug-toolbar.readthedocs.io/en/latest/changes.html#a1-2020-06-05 Assuming that perhaps this already had broken on earlier versions, i just as well wrote this patch with support for 3.0 instead. Conclusion: It might not actually work against djdt < 3.0.0 |
|
@aleksanb I think it may work with djdt 2.2 - I upgraded to 3.0a1 and am seeing the same grayed out behavior EDIT: Actually, turns out the styling of my site destroyed the checkbox I had to click to enable it. However, it appears blank - is this correct? |
the iframe should be filled with an enormous svg, if the script loaded correctly. I'm afraid that means you'll have to bump to 3.0. |
|
@blopker |
5 participants
Lifecycle methods changed on djdt 2.0 release, and scripts stopped
executing as well. This change requires you to add djdt_flamegraph to
INSTALLED_APPS.