Bug 16770 - Remove wrong uses of Memoize::Memcached
Summary: Remove wrong uses of Memoize::Memcached
Status: CLOSED 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:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-18 14:18 UTC by Jonathan Druart
Modified: 2017-12-07 22:16 UTC (History)
5 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 16770: Remove wrong caching of 3 subroutines in C4::Lancuages (1.34 KB, patch)
2016-06-18 14:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16770: Remove Memoize::Memcached dependency (2.97 KB, patch)
2016-06-18 14:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16770: Remove Memoize::Memcached dependency (2.97 KB, patch)
2016-06-18 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16770: Remove wrong caching of 3 subroutines in C4::Lancuages (1.40 KB, patch)
2016-06-19 17:27 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
Bug 16770: Remove wrong caching of 3 subroutines in C4::Lancuages (1.40 KB, patch)
2016-06-19 17:28 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
Bug 16770: Remove Memoize::Memcached dependency (3.16 KB, patch)
2016-06-19 17:28 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
Bug 16770: Remove 2 other occurrences of libmemoize-memcached-perl (1.29 KB, patch)
2016-06-21 11:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16770: Remove 2 other occurrences of libmemoize-memcached-perl (1.35 KB, patch)
2016-06-22 08:36 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
Bug 16770: Remove wrong caching of 3 subroutines in C4::Lancuages (1.45 KB, patch)
2016-06-23 13:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16770: Remove Memoize::Memcached dependency (3.20 KB, patch)
2016-06-23 13:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16770: Remove 2 other occurrences of libmemoize-memcached-perl (1.40 KB, patch)
2016-06-23 13:09 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 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 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 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 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 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