funkybob / django-sniplates

Template snippet libraries for Django
MIT License
57 stars 18 forks source link

Consider Debug toolbar support #53

Open kezabelle opened 7 years ago

kezabelle commented 7 years ago

Opening for discussion here, as I'm not on IRC right now. If I recall correctly, @funkybob, you're somehow wizard enough to not be a heavy user of django-debug-toolbar. I'm not, alas, and rely on it for sanity-checking/debugging. I'd like to explore/propose exposing enough hooks for the debug-toolbar template panel to pick up renderings that come out of sniplates. Specifically, currently, {% load_widgets %} calls out to resolve_blocks, which loads a template (via get_template) but never renders it, and because the API debug-toolbar expects a template to use (calling ._render on a Template instance) is never touched by sniplates, the template never ends up collected into the djdt panel. A naive solution to that is:

   ...
   # For Django 1.8 compatibility
    template = getattr(template, 'template', template)
    if settings.DEBUG:
        template_rendered.send(sender=None, template=template, context=context)

This would at least mean that templates loaded turn up in the templates list, which is a useful thing in and of itself, if you know that DTL (and subsequently often 3rd party tag libraries) will often try and continue vs erroring loudly.

To get individual block renderings would be slightly more work (but ultimately would be my preference, so that answering the questions like "what's in this form_field's context right now?" is easier for me), but also probably require deciding on how to handle any explicit djdt support anyway, hence this discussion :)

Basically:

funkybob commented 7 years ago

I'm happy to support DDT if it's not too invasive.

Is gating on DEBUG how DDT does it? I thought they had a more subtle system based on source IP?

sergei-maertens commented 7 years ago

The way I understand it the toolbar is only shown if all conditions are met, and it gathers info from listening to Django signals.

On Dec 3, 2016 00:16, "Curtis Maloney" notifications@github.com wrote:

I'm happy to support DDT if it's not too invasive.

Is gating on DEBUG how DDT does it? I thought they had a more subtle system based on source IP?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/funkybob/django-sniplates/issues/53#issuecomment-264588501, or mute the thread https://github.com/notifications/unsubscribe-auth/AFQ01kprHlWbKoar674IZe5nDI4bG1rjks5rEKbDgaJpZM4LCbkL .

funkybob commented 7 years ago

So you're saying it will gather stats anyway, but only show them for INTERNAL_IPs, etc?

sergei-maertens commented 7 years ago

DjDT operates via middleware. I think that middleware only kicks in if all the conditions are met, so that means that otherwise the signals would be fired, but there are no listeners outside of debug mode, and thus no stats gathering.

If this wasn't the case, it would also be invasive in core django, which I assume it isn't now.

That's a bunch of assumptions though, without checking the source of DjDT

On Dec 3, 2016 3:27 AM, "Curtis Maloney" notifications@github.com wrote:

So you're saying it will gather stats anyway, but only show them for INTERNAL_IPs, etc?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/funkybob/django-sniplates/issues/53#issuecomment-264609589, or mute the thread https://github.com/notifications/unsubscribe-auth/AFQ01rjW-_45gf5wD_Nmlb1PMX4nrkEfks5rENOXgaJpZM4LCbkL .

kezabelle commented 7 years ago

Sergei is right - for the intents and purposes of getting sniplates usage to turn up in the templates panel, it's enough to send the template_rendered signal which djdt (and any other tool) listens for, and then stuffs the data into threadlocal storage (I think...)

How djdt chooses to set itself up, historically and in the future, is thus it's own concern (and as you've rightly surmised, is as complex as it likes. IIRC it can auto-patch settings, looks for DEBUG, checks INTERNAL_IPS, and on top of that accepts a custom callable).

The notion of the settings.DEBUG guard conditional is in principle to avoid even sending a no-op signal event in production. It may or may not be a no-op in a developer's personal copy, depending on if something like djdt is in use and listening.

The signal being sent has to include an actual BaseContext subclass, and the template argument must have a name, an origin and an origin.name - for getting the individual template tags to register their usage/context, it would likely involve extracting the loadname etc from an Origin, but I'm not entirely sure there will always be one extractable from the working nodelist. It's something I need to dig into. This latter additional complexity is another reason that having a guard around the functionality would make sense (whether that be DEBUG or not).

funkybob commented 7 years ago

OK... sounds sufficient.

Now I'm wondering if we can play with how DDT displays information, to make it apparent this is a sniplate render, etc.

For now, I'm happy to accept a patch to add the gated signal, and we can look into more details later.

kezabelle commented 7 years ago

I'm not sure, but I suspect the best that could be done is to output the block/alias/whatever as the origin (rather than the full path in the case of a FS loader), because the template itself is rendered by an AJAX request on-demand, and I think that just re-opens the source for the template to render it as plaintext. More to look into :)