Bug 11167 - Remove memcached config from koha-conf-site.xml.in
Summary: Remove memcached config from koha-conf-site.xml.in
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Magnus Enger
QA Contact: Testopia
URL:
Keywords:
Depends on: 6193 10733
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-30 10:58 UTC by Magnus Enger
Modified: 2017-06-14 22:08 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11167 - Remove memcached config from koha-conf-site.xml.in (1.88 KB, patch)
2013-10-30 11:38 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 11167 - Remove memcached config from koha-conf-site.xml.in (1.89 KB, patch)
2013-10-31 00:46 UTC, Robin Sheat
Details | Diff | Splinter Review
Bug 11167 - Remove memcached config from koha-conf-site.xml.in (1.99 KB, patch)
2013-11-08 15:31 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 11167: QA-followup - C4::SQLHelper still used the koha-conf.xml entries (1.28 KB, patch)
2013-11-08 15:31 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2013-10-30 10:58:08 UTC
The file debian/templates/koha-conf-site.xml.in, which is used to generate koha-conf.xml files for packages installations, still contains a couple of lines for configuring memcached, even though this was moved to the Apache config files by bug 6193 (see specifically attachment 7630 [details] [review], which removes the relevant lines from /etc/koha-conf.xml). 

Here are the lines:
 <memcached_servers>__MEMCACHED_SERVERS__</memcached_servers>
 <memcached_namespace>__MEMCACHED_NAMESPACE__</memcached_namespace>

Having them there causes no harm, but it might create some confusion over where memcached is actually configured. 

This can also be seen as a followup to bug 10733 (attachment 21618 [details] [review]), which needlessly changes the lines mentioned above from:

- <memcached_servers></memcached_servers>
- <memcached_namespace></memcached_namespace>

to:

+ <memcached_servers>__MEMCACHED_SERVERS__</memcached_servers>
+ <memcached_namespace>__MEMCACHED_NAMESPACE__</memcached_namespace>
Comment 1 Magnus Enger 2013-10-30 11:38:45 UTC Comment hidden (obsolete)
Comment 2 Robin Sheat 2013-10-31 00:46:28 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2013-11-08 15:31:25 UTC
Created attachment 22828 [details] [review]
Bug 11167 - Remove memcached config from koha-conf-site.xml.in

Memcached-configuration is now done with environment variables
or SetEnv in the Apache configs. Having remnants of the now
obsolete configuration in koha-conf.xml might cause confusion as
to where memcached is actually configured. This patch removes the
two lines related to memcached from koha-conf-site.xml.in

To test, the hard way:
- Apply the patch
- Build packages
- Install package
- Create instances and enable memcached
- Check that memcached is used, even without the settings in
  koha-conf.xml

To test, the easy way:
- On an installation running off the packages, verify that memcached
  is actually configured through environment variables/SetEnv in the
  Apache configs, and not through koha-conf.xml

Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 4 Tomás Cohen Arazi 2013-11-08 15:31:43 UTC
Created attachment 22829 [details] [review]
Bug 11167: QA-followup - C4::SQLHelper still used the koha-conf.xml entries

There was still some use of the koha-conf.xml memcached_servers and
memcached_namespace configuration in C4::SQLHelper. This followup
removes that remaining bits.

Regards
To+

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 5 Galen Charlton 2014-05-05 22:04:10 UTC
I'm punting this one to later and revisit bug 6193 -- nowadays, I don't think it was a good idea to put the memcached config in the Apache configuration:

- passing environment variables to Plack is difficult
- we don't need to cache koha-conf.xml quite like this in memcached, avoiding the chicken-and-egg problem that may ahve motivated 6193.

Setting to in discussion.
Comment 6 Robin Sheat 2014-05-05 22:42:30 UTC
(In reply to Galen Charlton from comment #5)
> - passing environment variables to Plack is difficult

It's not really, I have the init script set to pull from a defaults file:

$ cat /etc/default/koha-common-starman 
# Defaults file for starman-related environment variables

# The location of the memcache server (currently only one can be provided.)
MEMCACHED_SERVERS=127.0.0.1:11211

# The namespace under which memcache keys will be stored.
# ${name} becomes the name of the instance.
MEMCACHED_NAMESPACE="${name}_koha"

# Koha debug options that you probably don't want to use unless you're trying
# to figure out a problem.
KOHA_BACKTRACE=1
DEBUG=1

> - we don't need to cache koha-conf.xml quite like this in memcached,
> avoiding the chicken-and-egg problem that may ahve motivated 6193.

We don't so much for plack, we can keep it in a local variable, and a read every 20 requests or whatever is no big deal. But it seems it'd shave a reasonable bit of time off loading the CGI implementation.