Summary: | Installer does not work under Plack | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | abl, dpavlin, gaetan.boisson, magnus, mirko, mtompset, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11998 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15562 |
||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 13791, 15973 | ||
Attachments: |
Bug 13805: Disable syspref cache - Plack
[SIGNED OFF] Bug 13805: Disable syspref cache - Plack |
Description
Jonathan Druart
2015-03-06 13:42:37 UTC
Created attachment 36706 [details] [review] Bug 13805: Disable syspref cache - Plack When going to the installer (without the DB structure), you have to log twice before starting the installation. Test plan: 0/ Create a new database and fill the database entry in the koha conf with its name 1/ Go on the mainpage, you should be redirected to the installer 2/ Try to log in Without this patch, you will get the login form again. With this patch, you can start the installation Actually the Dobrica's solution (on the wiki page) could be better: enable sub { my ( $app, $env ) = @_; return sub { my $env = shift; C4::Context->clear_syspref_cache(); $app->( $env ); } }; So, is the patch to be tested as is or going to be changed? The Dobrica's solution does not seem to work for this issue. I have the same double login issue. So, switching my patch back in the needs signof queue. I can't reproduce the problem on current master. I spin up kohadevbox, drop and recreate the database, then go to the webinstaller. After logging in once i can start to use the webinstaller. I should have said "then go to the webinstaller under Plack (port 5001)" Magnuse is running perl 5.14. I confirm the issue on current master, using perl 5.20 This also seems to work properly with perl 5.18 (In reply to Magnus Enger from comment #6) > I can't reproduce the problem on current master. I spin up kohadevbox, drop > and recreate the database, then go to the webinstaller. After logging in > once i can start to use the webinstaller. Really? After I upgrade to Jessie, and get everything working again... that was a mess... I get an error about $session being undefined on your kohadevbox. Created attachment 42431 [details] [review] [SIGNED OFF] Bug 13805: Disable syspref cache - Plack When going to the installer (without the DB structure), you have to log twice before starting the installation. Test plan: 0/ Create a new database and fill the database entry in the koha conf with its name 1/ Go on the mainpage, you should be redirected to the installer 2/ Try to log in Without this patch, you will get the login form again. With this patch, you can start the installation Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> This should be QAed please. It is not a blocker for 3.22 as the Plack integration it introduces is at package-level, but worth having both in sync. Patch pushed to master. Thanks Jonathan! This fix is not good for the performance. I have reverted it and cannot longer reproduce the original bug. Could someone confirm? (In reply to Jonathan Druart from comment #14) > This fix is not good for the performance. +1 And in some cases this is going to be _terrible_ for the performance - depending on the given script, and how many times it calls C4::Context->preference(), but there are some scripts and subroutines which tend to do it over and over and over again (1000+ calls per run being not all that uncommon, AFAIRC). Do we need to take another look at this? (In reply to Katrin Fischer from comment #16) > Do we need to take another look at this? Yes. Tomas, could you confirm that we don't need to disable this cache anymore? See bug 15973 for the revert. |