Bug 17274 - Add info about which memcached config is used to about.pl
Summary: Add info about which memcached config is used to about.pl
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 17261
Blocks:
  Show dependency treegraph
 
Reported: 2016-09-08 09:26 UTC by Jonathan Druart
Modified: 2017-06-14 22:18 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
The memcached config can be read from ENV or koha-conf.xml In order to remove any ambiguity, now we inform the user which config is used in About Koha > Server information page.


Attachments
Bug 17274: Display the place where the memcached is picked (3.87 KB, patch)
2016-09-08 09:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17274: Display the place where the memcached is picked (3.94 KB, patch)
2016-09-12 06:45 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 17274: Display the place where the memcached is picked (4.01 KB, patch)
2016-11-01 19:05 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 Jonathan Druart 2016-09-08 09:26:28 UTC
The memcached config can be read from ENV or koha-conf.xml
In order to remove any ambiguity, let's inform the user which config is used.
Comment 1 Jonathan Druart 2016-09-08 09:29:12 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2016-09-09 13:00:39 UTC
Jonathan: Maybe it is better to keep warnings on 'System information'?
Comment 3 Marc Véron 2016-09-12 06:44:09 UTC
(In reply to Tomás Cohen Arazi from comment #2)
> Jonathan: Maybe it is better to keep warnings on 'System information'?

The place where the warning is displayed (i.e. same place as info about memcached configuration / Bug 17261) makes sense to me.
Comment 4 Marc Véron 2016-09-12 06:45:45 UTC
Created attachment 55479 [details] [review]
Bug 17274: Display the place where the memcached is picked

The memcached config can be read from ENV or koha-conf.xml
In order to remove any ambiguity, let's inform the user which config is
used.

Test plan:
1/ Define the memcached config in ENV and $KOHA_CONF
=> Go on about page, you should get a warning
2/ Define the memcached config in ENV and not in $KOHA_CONF
=> Go on about page, you should get a warning
3/ Do not defined the memcached config in ENV or $KOHA_CONF
=> Go on about page, you should get a warning
4/ Define the memcached config in $KOHA_CONF and not in ENV
=> Go on about page, you should not get a warning, this is the expected
config

Followed test plan, works as expected.
Signed-off-by: Marc <veron@veron.ch>
Comment 5 Frédéric Demians 2016-09-21 17:54:59 UTC
Where is memcached server extracted from koha-conf.xml? I don't see it in Koha::Cache.pm.
Comment 6 Tomás Cohen Arazi 2016-09-21 17:58:42 UTC
(In reply to Frédéric Demians from comment #5)
> Where is memcached server extracted from koha-conf.xml? I don't see it in
> Koha::Cache.pm.



koha::Cache.pm (86~93):
    $self->{'namespace'} ||= $ENV{MEMCACHED_NAMESPACE};
    my @servers = split /,/, $ENV{MEMCACHED_SERVERS} || '';
    unless ( $self->{namespace} and @servers ) {
        my $koha_config = Koha::Config->read_from_file( Koha::Config->guess_koha_conf() );
        $self->{namespace} ||= $koha_config->{config}{memcached_namespace} || 'koha';
        @servers = split /,/, $koha_config->{config}{memcached_servers} // ''
            unless @servers;
    }
Comment 7 Tomás Cohen Arazi 2016-09-21 18:00:41 UTC
(In reply to Frédéric Demians from comment #5)
> Where is memcached server extracted from koha-conf.xml? I don't see it in
> Koha::Cache.pm.

In current master, it reads it from koha-conf.xml BUT it prefers ENV if available. That's why I mentioned it is safe for stable.
Comment 8 Katrin Fischer 2016-11-01 19:05:16 UTC
Created attachment 57056 [details] [review]
[PASSED QA] Bug 17274: Display the place where the memcached is picked

The memcached config can be read from ENV or koha-conf.xml
In order to remove any ambiguity, let's inform the user which config is
used.

Test plan:
1/ Define the memcached config in ENV and $KOHA_CONF
=> Go on about page, you should get a warning
2/ Define the memcached config in ENV and not in $KOHA_CONF
=> Go on about page, you should get a warning
3/ Do not defined the memcached config in ENV or $KOHA_CONF
=> Go on about page, you should get a warning
4/ Define the memcached config in $KOHA_CONF and not in ENV
=> Go on about page, you should not get a warning, this is the expected
config

Followed test plan, works as expected.
Signed-off-by: Marc <veron@veron.ch>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 9 Kyle M Hall 2016-11-02 10:56:10 UTC
Pushed to master for 16.11, thanks Jonathan!
Comment 10 Mason James 2016-11-30 08:17:12 UTC
enhancement, passing for 16.05