Bug 16770

Summary: Remove wrong uses of Memoize::Memcached
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: abl, f.demians, julian.maurice, kyle, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16579
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7879
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 16770: Remove wrong caching of 3 subroutines in C4::Lancuages
Bug 16770: Remove Memoize::Memcached dependency
Bug 16770: Remove Memoize::Memcached dependency
Bug 16770: Remove wrong caching of 3 subroutines in C4::Lancuages
Bug 16770: Remove wrong caching of 3 subroutines in C4::Lancuages
Bug 16770: Remove Memoize::Memcached dependency
Bug 16770: Remove 2 other occurrences of libmemoize-memcached-perl
Bug 16770: Remove 2 other occurrences of libmemoize-memcached-perl
Bug 16770: Remove wrong caching of 3 subroutines in C4::Lancuages
Bug 16770: Remove Memoize::Memcached dependency
Bug 16770: Remove 2 other occurrences of libmemoize-memcached-perl

Description Jonathan Druart 2016-06-18 14:18:39 UTC
From C4/languages.pm:
 32 eval { 
 33     if (C4::Context->ismemcached) {
 34         require Memoize::Memcached;
 35         import Memoize::Memcached qw(memoize_memcached);
 36         
 37         memoize_memcached('getTranslatedLanguages', memcached => C4::Context->memcached);
 38         memoize_memcached('getFrameworkLanguages' , memcached => C4::Context->memcached);
 39         memoize_memcached('getAllLanguages',        memcached => C4::Context->memcached);
 40     }
 41 };

Looking at this code, you might think these subroutines are cached, but actually they are not.

The eval surrounding the code hides a bug, if you remove it, you will get:
  Invalid memcached argument (expected a hash)
Comment 1 Jonathan Druart 2016-06-18 14:27:07 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-06-18 14:27:10 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-06-18 14:29:30 UTC Comment hidden (obsolete)
Comment 4 Jacek Ablewicz 2016-06-19 17:27:12 UTC Comment hidden (obsolete)
Comment 5 Jacek Ablewicz 2016-06-19 17:28:04 UTC Comment hidden (obsolete)
Comment 6 Jacek Ablewicz 2016-06-19 17:28:10 UTC Comment hidden (obsolete)
Comment 7 Jacek Ablewicz 2016-06-19 17:33:17 UTC
Some references to 'memoize' are still left in 

./install_misc/ubuntu.10.04.packages:libmemoize-memcached-perl install
./install_misc/ubuntu.12.04.packages:libmemoize-memcached-perl install

not sure if those files (whatever they are) are still relevent or not?
Comment 8 Jonathan Druart 2016-06-21 11:37:11 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2016-06-21 11:37:33 UTC
(In reply to Jacek Ablewicz from comment #7)
> Some references to 'memoize' are still left in 
> 
> ./install_misc/ubuntu.10.04.packages:libmemoize-memcached-perl install
> ./install_misc/ubuntu.12.04.packages:libmemoize-memcached-perl install
> 
> not sure if those files (whatever they are) are still relevent or not?

They should be removed as well indeed.
Comment 10 Jacek Ablewicz 2016-06-22 08:36:28 UTC Comment hidden (obsolete)
Comment 11 Tomás Cohen Arazi (tcohen) 2016-06-23 13:09:11 UTC
Created attachment 52712 [details] [review]
Bug 16770: Remove wrong caching of 3 subroutines in C4::Lancuages

ooking at this code, you might think these subroutines are cached, but
actually they are not.

The eval surrounding the code hides a bug, if you remove it, you will
get:
  Invalid memcached argument (expected a hash)

Test plan:
Do not apply this patch and confirm that the code does not work

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi (tcohen) 2016-06-23 13:09:18 UTC
Created attachment 52713 [details] [review]
Bug 16770: Remove Memoize::Memcached dependency

This module is no longer in use and can be removed.

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>

This part involves some changes in a bunch of mysterious debian|ubuntu
related files, not quite sure if I know what I'm doing
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi (tcohen) 2016-06-23 13:09:23 UTC
Created attachment 52714 [details] [review]
Bug 16770: Remove 2 other occurrences of libmemoize-memcached-perl

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Kyle M Hall (khall) 2016-06-24 14:05:58 UTC
Pushed to master for 16.11, thanks Jonathan!
Comment 15 Frédéric Demians 2016-08-01 13:19:30 UTC
Pushed in 16.05. Will be in 16.05.02.
Comment 16 Julian Maurice 2016-08-16 13:37:44 UTC
Patches pushed to 3.22.x, will be in 3.22.10