Bug 17951 - koha-create should create the template cache dir and configure it in koha-conf.xml
Summary: koha-create should create the template cache dir and configure it in koha-con...
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Whiteboard:
Keywords: rel_17_11_candidate
: 17957 (view as bug list)
Depends on: 7533
Blocks:
  Show dependency treegraph
 
Reported: 2017-01-20 14:35 UTC by Tomás Cohen Arazi
Modified: 2017-10-16 12:21 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17951: Make koha-create set template_cache_dir correctly (8.91 KB, patch)
2017-01-24 16:06 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17951: Make koha-create set template_cache_dir correctly (8.97 KB, patch)
2017-04-20 06:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17951: (followup) Make Makefile.PL handle template_cache_dir config (5.13 KB, patch)
2017-04-25 15:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17951: Make koha-create set template_cache_dir correctly (9.00 KB, patch)
2017-10-15 12:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17951: (followup) Make Makefile.PL handle template_cache_dir config (5.19 KB, patch)
2017-10-15 12:17 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2017-01-20 14:35:04 UTC

    
Comment 1 Jonathan Druart 2017-01-23 07:57:51 UTC
*** Bug 17957 has been marked as a duplicate of this bug. ***
Comment 2 Tomás Cohen Arazi 2017-01-24 16:06:14 UTC
Created attachment 59512 [details] [review]
Bug 17951: Make koha-create set template_cache_dir correctly

This patch makes koha-create (and friends) handle the template_cache_dir
config entry correctly.

It does so by:
- Adding a replaceable string to the template for koha-conf.xml
- Making koha-create-dirs create the needed directories (i.e.
 /var/cache/koha/instance and /var/cache/koha/instance/templates)
- Adding a --template-cache-dir switch to koha-create (so sysadmins can
  specify their favourite directory for the templates cache).
- koha-remove now takes care of the instance's *templates* dir.
- The install scripts now automatically create /var/cache/koha so it can
  be used later by koha-create and friends. It does so the same way it does
  for other install-created directories.

To test, you should ideally be able to build your own packages. This
instructions can be followed by people that doesn't have that ability
yet. But can be used on a custom packages setup too.

To test:
- Make sure you have the latest misc4dev in your kohadevbox (if it is a
  fresh box you have it already)
- Run:
  $ sudo perl misc4dev/cp_debian_files.pl
- Manually create the /var/cache/koha dir (skip if you have your own
        packages):
  $ sudo mkdir /var/cache/koha
- Create a new instance:
  $ sudo koha-create --create-db cachetest
=> SUCCESS:
    * /etc/koha/sites/cachetest/koha-conf.xml contains
    template_cache_dir and is populated with /var/cache/koha/cachetest/templates
    * The directory /var/cache/koha/cachetest/templates exists!
- Create a new instance, pass your own cache dir:
  $ sudo koha-create --create-db --template-cache-dir /tmp cachetest2
=> SUCCESS: etc/koha/sites/cachetest2/koha-conf.xml contains template_cache_dir
   and is populated with /tmp
- Run:
  $ man koha-create
=> SUCCESS: The docs mention the --template-cache-dir option switch
    correctly.
- Sign off :-D!
Comment 3 Marcel de Rooy 2017-04-19 15:28:36 UTC
Works fine. Will be signing off tomorrow. And mentioning a few issues we might still consider.
Comment 4 Marcel de Rooy 2017-04-20 06:45:54 UTC
Created attachment 62443 [details] [review]
Bug 17951: Make koha-create set template_cache_dir correctly

This patch makes koha-create (and friends) handle the template_cache_dir
config entry correctly.

It does so by:
- Adding a replaceable string to the template for koha-conf.xml
- Making koha-create-dirs create the needed directories (i.e.
 /var/cache/koha/instance and /var/cache/koha/instance/templates)
- Adding a --template-cache-dir switch to koha-create (so sysadmins can
  specify their favourite directory for the templates cache).
- koha-remove now takes care of the instance's *templates* dir.
- The install scripts now automatically create /var/cache/koha so it can
  be used later by koha-create and friends. It does so the same way it does
  for other install-created directories.

To test, you should ideally be able to build your own packages. This
instructions can be followed by people that doesn't have that ability
yet. But can be used on a custom packages setup too.

To test:
- Make sure you have the latest misc4dev in your kohadevbox (if it is a
  fresh box you have it already)
- Run:
  $ sudo perl misc4dev/cp_debian_files.pl
- Manually create the /var/cache/koha dir (skip if you have your own
        packages):
  $ sudo mkdir /var/cache/koha
- Create a new instance:
  $ sudo koha-create --create-db cachetest
=> SUCCESS:
    * /etc/koha/sites/cachetest/koha-conf.xml contains
    template_cache_dir and is populated with /var/cache/koha/cachetest/templates
    * The directory /var/cache/koha/cachetest/templates exists!
- Create a new instance, pass your own cache dir:
  $ sudo koha-create --create-db --template-cache-dir /tmp cachetest2
=> SUCCESS: etc/koha/sites/cachetest2/koha-conf.xml contains template_cache_dir
   and is populated with /tmp
- Run:
  $ man koha-create
=> SUCCESS: The docs mention the --template-cache-dir option switch
    correctly.
- Sign off :-D!

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2017-04-20 06:55:36 UTC
Still want to mention:

[1] Should koha-remove remove a custom template cache dir? It currently does not and this is consistent with the way we handle other dirs like uploads. Although template cache files are better candidates for immediate removal.

[2] Should we somehow allow a user to say that he does not want template caching? I think that it is good to enable template caching by default now in packaging, but should we make it easier to disable it ?
Comment 6 Jonathan Druart 2017-04-24 19:40:04 UTC
Sounds like changes to etc/koha-conf.xml are missing.
Comment 7 Tomás Cohen Arazi 2017-04-24 19:42:38 UTC
(In reply to Jonathan Druart from comment #6)
> Sounds like changes to etc/koha-conf.xml are missing.

Ha, it is there
Comment 8 Tomás Cohen Arazi 2017-04-25 00:42:32 UTC
(In reply to Jonathan Druart from comment #6)
> Sounds like changes to etc/koha-conf.xml are missing.

Bug 7533?
Comment 9 Jonathan Druart 2017-04-25 12:38:36 UTC
(In reply to Tomás Cohen Arazi from comment #8)
> (In reply to Jonathan Druart from comment #6)
> > Sounds like changes to etc/koha-conf.xml are missing.
> 
> Bug 7533?

I do not understand what you are referring to. I am aware of bug 7533 as I wrote the patch for it.

Apply the patch then compare etc/koha-conf.xml and debian/templates/koha-conf-site.xml.in, I think you will understand I am saying.
Looking at bug 16083 it seems that we should continue to keep them in sync.

Do not you think?
Comment 10 Tomás Cohen Arazi 2017-04-25 14:25:03 UTC
(In reply to Marcel de Rooy from comment #5)
> Still want to mention:
> 
> [1] Should koha-remove remove a custom template cache dir? It currently does
> not and this is consistent with the way we handle other dirs like uploads.
> Although template cache files are better candidates for immediate removal.

I mimicked what is done with uploads. I think a custom dir shouldn't be removed.

> [2] Should we somehow allow a user to say that he does not want template
> caching? I think that it is good to enable template caching by default now
> in packaging, but should we make it easier to disable it?

If a user wants to disable template caching, they just need to comment out the relevant entry. But they will still get the warning in about.pl, that originally triggered me to write this patch to start with (i.e. we introduced something that will always warn the user if an entry is missing, and not add it automatically in the first place).
Comment 11 Tomás Cohen Arazi 2017-04-25 14:27:51 UTC
(In reply to Jonathan Druart from comment #9)
> (In reply to Tomás Cohen Arazi from comment #8)
> > (In reply to Jonathan Druart from comment #6)
> > > Sounds like changes to etc/koha-conf.xml are missing.
> > 
> > Bug 7533?
> 
> I do not understand what you are referring to. I am aware of bug 7533 as I
> wrote the patch for it.

Yes, I misunderstood you at the beginning of this conversation. Sorry!

> Apply the patch then compare etc/koha-conf.xml and
> debian/templates/koha-conf-site.xml.in, I think you will understand I am
> saying.

I now understand you want koha-conf.xml to be filled with the same stuff, enabled by default, etc.

> Looking at bug 16083 it seems that we should continue to keep them in sync.

Wow, I always thought I was aware of developments in the install/CLI stuff, and now I know I'm not aware of all of them :-D

> Do not you think?

Yes, they should be in sync.
Comment 12 Tomás Cohen Arazi 2017-04-25 15:36:31 UTC
Created attachment 62690 [details] [review]
Bug 17951: (followup) Make Makefile.PL handle template_cache_dir config

This patch adds Makefile.PL the capability of handling the template_cache_dir configuration entry.
To do so, it:
- Adds the --template-cache-dir option switch (consistency with koha-create)
- Sets a default value for template_cache_dir to '/tmp/koha'
- Adds a dialog requesting the path for the template cache dir to Makefile.PL
- It tweaks etc/koha-conf.xml so it is correctly changed by rewrite-config.PL

To test:
- Apply this patch
- Run:
  $ perl Makefile.PL --template-cache-dir your/favourite/dir
=> SUCCESS: The dialogs don't ask for template cache dir
=> SUCCESS: The resulting Makefile contains an entry for TEMPLATE_CACHE_DIR which value
            matches what we passed to --template-cache-dir
- Run:
  $ perl Makefile.PL
- When prompted for a template cache dir, introduce whatever you want
=> SUCCESS: The default you are offered is /tmp/koha
=> SUCCESS: At the end of the process, Makefile contains what we put in there
- Run:
  $ sudo make install
=> SUCCESS: The resulting koha-conf.xml contains a <template_cache_dir> entry containing
            whatever you picked for that purpose.
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Katrin Fischer 2017-10-15 12:17:46 UTC
Created attachment 68163 [details] [review]
Bug 17951: Make koha-create set template_cache_dir correctly

This patch makes koha-create (and friends) handle the template_cache_dir
config entry correctly.

It does so by:
- Adding a replaceable string to the template for koha-conf.xml
- Making koha-create-dirs create the needed directories (i.e.
 /var/cache/koha/instance and /var/cache/koha/instance/templates)
- Adding a --template-cache-dir switch to koha-create (so sysadmins can
  specify their favourite directory for the templates cache).
- koha-remove now takes care of the instance's *templates* dir.
- The install scripts now automatically create /var/cache/koha so it can
  be used later by koha-create and friends. It does so the same way it does
  for other install-created directories.

To test, you should ideally be able to build your own packages. This
instructions can be followed by people that doesn't have that ability
yet. But can be used on a custom packages setup too.

To test:
- Make sure you have the latest misc4dev in your kohadevbox (if it is a
  fresh box you have it already)
- Run:
  $ sudo perl misc4dev/cp_debian_files.pl
- Manually create the /var/cache/koha dir (skip if you have your own
        packages):
  $ sudo mkdir /var/cache/koha
- Create a new instance:
  $ sudo koha-create --create-db cachetest
=> SUCCESS:
    * /etc/koha/sites/cachetest/koha-conf.xml contains
    template_cache_dir and is populated with /var/cache/koha/cachetest/templates
    * The directory /var/cache/koha/cachetest/templates exists!
- Create a new instance, pass your own cache dir:
  $ sudo koha-create --create-db --template-cache-dir /tmp cachetest2
=> SUCCESS: etc/koha/sites/cachetest2/koha-conf.xml contains template_cache_dir
   and is populated with /tmp
- Run:
  $ man koha-create
=> SUCCESS: The docs mention the --template-cache-dir option switch
    correctly.
- Sign off :-D!

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2017-10-15 12:17:50 UTC
Created attachment 68164 [details] [review]
Bug 17951: (followup) Make Makefile.PL handle template_cache_dir config

This patch adds Makefile.PL the capability of handling the template_cache_dir configuration entry.
To do so, it:
- Adds the --template-cache-dir option switch (consistency with koha-create)
- Sets a default value for template_cache_dir to '/tmp/koha'
- Adds a dialog requesting the path for the template cache dir to Makefile.PL
- It tweaks etc/koha-conf.xml so it is correctly changed by rewrite-config.PL

To test:
- Apply this patch
- Run:
  $ perl Makefile.PL --template-cache-dir your/favourite/dir
=> SUCCESS: The dialogs don't ask for template cache dir
=> SUCCESS: The resulting Makefile contains an entry for TEMPLATE_CACHE_DIR which value
            matches what we passed to --template-cache-dir
- Run:
  $ perl Makefile.PL
- When prompted for a template cache dir, introduce whatever you want
=> SUCCESS: The default you are offered is /tmp/koha
=> SUCCESS: At the end of the process, Makefile contains what we put in there
- Run:
  $ sudo make install
=> SUCCESS: The resulting koha-conf.xml contains a <template_cache_dir> entry containing
            whatever you picked for that purpose.
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>