Our extensive hacking to Plack::Middleware::Debug

36 views
Skip to first unread message

Ævar Arnfjörð Bjarmason

unread,
May 1, 2014, 12:01:12 PM5/1/14
to Marcel Gruenauer, Tatsuhiko Miyagawa, psgi-...@googlegroups.com
At work we have some extensive monkeypatches to
Plack::Middleware::Debug, basically:

* Panels load asynchronously, their content is just "Loading..." and
on clicking them a JS loads them from
/debug_panel_content?id=$UUID_for_request
* This allows us to populate panels with data that's only generated
in the cleanup phase of the request
* We've added various tools to do prettier rendering / searching of
JSON data we inject into the panels

And we were going to sometime soon add support for:

* nav_subtitle taking a CODE reference, which could be populated
after the cleanup phase, then on load the page would poll a URL to
populate the subtitles (to e.g. say "cleanup phase = had errors")
* Panels could declare that they had issues ($panel->grumble("look at
me")), so even if they're collapsed users would go and look at them
* Panels could also declare whether they could run entirely in the
main request, or needed to execute after the cleanup phase

And maybe even:

* Don't actually generate any panel data for some expensive (e.g.
NYTProf) panels, but when they're clicked on dispatch another request
with the same parameters that would generate that.

I think it would be cool if we retained API compatibility with
upstream while doing this, it's also neat to be able to use the
upstream panels.

But obviously this makes the implementation quite a bit more complex,
panels aren't all that stand-alone anymore, but also require the
server to being able to serve up files asynchronously on-disk. It'll
also have to do things like interact with the cleanup phase, and at
least due to the stuff we need (executing in the middle of another
cleanup phase) it wouldn't be a very pretty API.

So we're basically at the point where either we just fully fork the
thing, or push patches to upstream that optionally turn on a bunch of
horror via optional options.

It's easy for us to just maintain this fork, and I'm not all that
interested in poking at pushing it upstream unless stuff like this
might actually get accepted, and people might use it (does anyone else
have huge expensive pavels, and use the cleanup phase extensively?).

So, what do you think?

Tatsuhiko Miyagawa

unread,
May 1, 2014, 12:20:57 PM5/1/14
to psgi-...@googlegroups.com

Forking it and giving it a separate name sounds all cool to me. A compatibility with existing panels would be nice to have.

Sent from mobile

--

---
You received this message because you are subscribed to the Google Groups "psgi-plack" group.
To unsubscribe from this group and stop receiving emails from it, send an email to psgi-plack+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Ævar Arnfjörð Bjarmason

unread,
May 1, 2014, 1:47:04 PM5/1/14
to psgi-...@googlegroups.com

Is that another way of saying you wouldn't accept those patches in the main distro? :)

I can upload a fork, but I'd rather just patch P::M::Debug with some opt-in options if possible.

Tatsuhiko Miyagawa

unread,
May 1, 2014, 1:52:01 PM5/1/14
to psgi-...@googlegroups.com

You didn't include a link to the code so there isn't much to say but the list looks extensive and complex to add to the existing one.

More importantly you guys seem to use it more widely and are enthusiastic about extending it which makes a fork more reasonable since making it into one distro would force you to care about the backward compat. :)

Ævar Arnfjörð Bjarmason

unread,
May 1, 2014, 2:47:42 PM5/1/14
to psgi-...@googlegroups.com
On Thu, May 1, 2014 at 7:52 PM, Tatsuhiko Miyagawa <miya...@gmail.com> wrote:
> You didn't include a link to the code so there isn't much to say but the
> list looks extensive and complex to add to the existing one.
>
> More importantly you guys seem to use it more widely and are enthusiastic
> about extending it which makes a fork more reasonable since making it into
> one distro would force you to care about the backward compat. :)

Of course I'm not expecting you to accept a patch you haven't seen.
I'm just asking if you're in principle opposed to accepting a patch
that adds a bunch of new optional options to Plack::Middleware::Debug
to support this kind of thing.

The hacks we have are entirely compatible with existing middleware,
we're just adding extra $panel->$whatever() methods when needed.

Tatsuhiko Miyagawa

unread,
May 1, 2014, 5:24:16 PM5/1/14
to psgi-...@googlegroups.com
Well I think it's dangerous to say yes to a patch that hasn't been written/seen, since turning that down then will let you down, or being obliged to merge them will increase the burden of maintaining the code base in the future.

Hence usually my canned answer to feature requests is No - especially when it comes to Plack middleware where you can just replace the name of middleware to the new one. 
Tatsuhiko Miyagawa
Reply all
Reply to author
Forward
0 new messages