[Bug 41896] New: Remove C4::Context from Koha::Cache
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 Bug ID: 41896 Summary: Remove C4::Context from Koha::Cache Initiative type: --- Sponsorship --- status: Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 41890 Koha::Cache uses C4::Context that uses Koha::Caches (and so Koha::Cache). Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41890 [Bug 41890] [OMNIBUS] Remove circular dependencies -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 --- Comment #1 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 193510 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193510&action=edit Bug 41896: Remove C4::Context from Koha::Cache Patch from commit 97f28e6 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #2 from David Cook <dcook@prosentient.com.au> --- This one looks pretty straightforward. Solid change. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #193510|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 --- Comment #3 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 196218 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196218&action=edit Bug 41896: Remove C4::Context from Koha::Cache Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off CC| |julian.maurice@biblibre.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 --- Comment #4 from Julian Maurice <julian.maurice@biblibre.com> --- Simple change. Memcached cache continues to work as usual -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|41890 | Blocks| |41890 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41890 [Bug 41890] [OMNIBUS] Remove circular dependencies -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jonathan.druart@gmail.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42663 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42663 [Bug 42663] Koha::Context must be object-oriented designed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|41890 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41890 [Bug 41890] [OMNIBUS] Remove circular dependencies -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 196218 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196218 Bug 41896: Remove C4::Context from Koha::Cache Review of attachment 196218: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=41896&attachment=196218) ----------------------------------------------------------------- ::: Koha/Cache.pm @@ +75,4 @@
# Should we continue to support MEMCACHED ENV vars? $self->{'namespace'} ||= $ENV{MEMCACHED_NAMESPACE}; my @servers = split /,/, $ENV{MEMCACHED_SERVERS} || ''; + $self->{namespace} = Koha::Config->get_instance->get('memcached_namespace') || 'koha';
By replacing ||= with = we're making the above checking of the MEMCACHED_NAMESPACE environmental variable pointless. Are we sure we want to do that? If so, then we should remove support for those ENV variables. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #5)
By replacing ||= with = we're making the above checking of the MEMCACHED_NAMESPACE environmental variable pointless. Are we sure we want to do that? If so, then we should remove support for those ENV variables.
about.pl also seems to support an environmental variable for MEMCACHED_NAMESPACE. I think that's for the old school Apache config. Shall we just remove these two references? about.pl: my $memcached_namespace = $ENV{MEMCACHED_NAMESPACE} || C4::Context->config('memcached_namespace') // 'koha'; Koha/Cache.pm: $self->{'namespace'} ||= $ENV{MEMCACHED_NAMESPACE}; Likewise MEMCACHED_SERVERS only seems to be supported in about.pl and Koha/Cache.pm. (Looks like debian/scripts/koha-create and debian/templates/koha-sites.conf work together a bit differently than a person might expect...) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off --- Comment #7 from David Cook <dcook@prosentient.com.au> --- Actually, don't worry about it. I'll open a different bug for that, so that we can keep moving with this one, which is a very sensible change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=42668 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #8 from David Cook <dcook@prosentient.com.au> --- Ahhh then again... It's like a 2 character change. Let's keep the status quo for now, unless we want to do bug 42668 first. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196218|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 --- Comment #9 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 199533 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199533&action=edit Bug 41896: Remove C4::Context from Koha::Cache Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Amended-by: Jonathan Druart Restore ||= op - $self->{namespace} = Koha::Config->get_instance->get('memcached_namespace') || 'koha'; + $self->{namespace} ||= Koha::Config->get_instance->get('memcached_namespace') || 'koha'; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 --- Comment #10 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to David Cook from comment #5)
Comment on attachment 196218 [details] [review] Bug 41896: Remove C4::Context from Koha::Cache
Review of attachment 196218 [details] [review]: -----------------------------------------------------------------
::: Koha/Cache.pm @@ +75,4 @@
# Should we continue to support MEMCACHED ENV vars? $self->{'namespace'} ||= $ENV{MEMCACHED_NAMESPACE}; my @servers = split /,/, $ENV{MEMCACHED_SERVERS} || ''; + $self->{namespace} = Koha::Config->get_instance->get('memcached_namespace') || 'koha';
By replacing ||= with = we're making the above checking of the MEMCACHED_NAMESPACE environmental variable pointless. Are we sure we want to do that? If so, then we should remove support for those ENV variables.
No idea why I've done that! I've restored ||= -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41896 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org