Created attachment 6959 [details] [review] Bug 7387 : Adding a caching plugin for Template::Toolkit And an example of how to use it
Created attachment 7368 [details] [review] Bug 7387 : Adding a caching plugin for Template::Toolkit And an example of how to use it Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Setting this one to "in discussion", as it's linked to bug 7248 QA comment: chris, there is no explanation of the positive effect of this. What would be the next step if this patch is included ? could we do that with all includes ? more explanation welcomed !
I remove the "in discussion" status. This patch is very interesting, but bug 7248 does not apply anymore, so this patch can't apply until 7248 has been rewritten.
Bug 7248 is pushed, I switch this one to "Signed off"
Mostly just adding the new plugin, which appears to be functional. Example of usage in intranet-main, to get the doc-head-open.inc for 6 minutes. Should be an easy enough thing to revert if we later find some deep dark nastiness, but I can't see any right now, and will mark Passed QA. Note that 3-way merge is required... git bz is not applying for me here directly.
(In reply to comment #3) > QA comment: chris, there is no explanation of the positive effect of this. > What would be the next step if this patch is included ? could we do that > with all includes ? more explanation welcomed ! Chris, before pushing this patch, i'd like to understand what will be the next step. Should all templates be switched to: -[% INCLUDE 'doc-head-open.inc' %] +[% USE cache = Cache %] +[% cache.inc( + 'template' => 'doc-head-open.inc', + 'keys' => {'dochead' => 'dochead'}, + 'ttl' => 360 + ) +%] ? (if that's the case, then I'll add this to my next RM newsletter & send a mail to koha-devel about that) Do you already have a list of other .inc we could cache too ?
(In reply to comment #7) > (In reply to comment #3) > > QA comment: chris, there is no explanation of the positive effect of this. > > What would be the next step if this patch is included ? could we do that > > with all includes ? more explanation welcomed ! > > Chris, before pushing this patch, i'd like to understand what will be the > next step. Should all templates be switched to: > -[% INCLUDE 'doc-head-open.inc' %] > +[% USE cache = Cache %] > +[% cache.inc( > + 'template' => 'doc-head-open.inc', > + 'keys' => {'dochead' => 'dochead'}, > + 'ttl' => 360 > + ) > +%] > > ? > (if that's the case, then I'll add this to my next RM newsletter & send a > mail to koha-devel about that) > Do you already have a list of other .inc we could cache too ? Nope d, in fact caching that one won't win us much at all it's fast to render anyway. But looking into caching things that require computation, would be good. This patch is just to enable people to cache includes when they find ones, or even better create includes and cache them.
(In reply to comment #8) > Nope d, in fact caching that one won't win us much at all it's fast to > render anyway. But looking into caching things that require computation, > would be good. This patch is just to enable people to cache includes when > they find ones, or even better create includes and cache them. mmm... I'm not sure this patch should be pushed as is : if it's a good idea, the -[% INCLUDE 'doc-head-open.inc' %] +[% USE cache = Cache %] +[% cache.inc( + 'template' => 'doc-head-open.inc', + 'keys' => {'dochead' => 'dochead'}, + 'ttl' => 360 + ) +%] is useless, so should be removed, and a more useful example used ? QA comment: isn't {'cache_servers' => C4::Context->config('memcached_servers') } as memcache is the default server for Koha::Cache ? marking failed QA for now
Created attachment 14358 [details] [review] Bug 7387 : Adding a caching plugin for Template::Toolkit To use this you need to do something like -[% INCLUDE 'doc-head-open.inc' %] +[% USE cache = Cache %] +[% cache.inc( + 'template' => 'doc-head-open.inc', + 'keys' => {'dochead' => 'dochead'}, + 'ttl' => 360 + ) +%] Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 15502 [details] [review] [SIGNED-OFF] Bug 7387 : Adding a caching plugin for Template::Toolkit To use this you need to do something like -[% INCLUDE 'doc-head-open.inc' %] +[% USE cache = Cache %] +[% cache.inc( + 'template' => 'doc-head-open.inc', + 'keys' => {'dochead' => 'dochead'}, + 'ttl' => 360 + ) +%] Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: Works. Tested modifying as described opac-main.tt Follow up to fix tabulations.
Created attachment 15503 [details] [review] Follow-up Bug 7387 - Add Template::Toolkit plugin to allow caching of includes Fixes tabulations.
Created attachment 15892 [details] [review] Bug 7387 : Adding a caching plugin for Template::Toolkit To use this you need to do something like -[% INCLUDE 'doc-head-open.inc' %] +[% USE cache = Cache %] +[% cache.inc( + 'template' => 'doc-head-open.inc', + 'keys' => {'dochead' => 'dochead'}, + 'ttl' => 360 + ) +%] Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: Works. Tested modifying as described opac-main.tt Follow up to fix tabulations. Signed-off-by: Elliott Davis <elliott@bywatersolions.com>
Created attachment 15893 [details] [review] Follow-up Bug 7387 - Add Template::Toolkit plugin to allow caching of includes Fixes tabulations. Signed-off-by: Elliott Davis <elliott@bywatersolions.com>
This patch has been pushed to master.