Bug 13805 - Installer does not work under Plack
Summary: Installer does not work under Plack
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 13791 15973
  Show dependency treegraph
 
Reported: 2015-03-06 13:42 UTC by Jonathan Druart
Modified: 2017-06-14 22:11 UTC (History)
8 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:


Attachments
Bug 13805: Disable syspref cache - Plack (1.09 KB, patch)
2015-03-06 13:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED OFF] Bug 13805: Disable syspref cache - Plack (1.16 KB, patch)
2015-09-07 13:42 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 2015-03-06 13:42:37 UTC
When going to the installer (without the DB structure), you have to log twice before starting the installation.
Comment 1 Jonathan Druart 2015-03-06 13:46:50 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-03-06 13:52:37 UTC
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 );
        }
    };
Comment 3 Mirko Tietgen 2015-03-11 18:31:54 UTC
So, is the patch to be tested as is or going to be changed?
Comment 4 Jonathan Druart 2015-03-12 08:15:57 UTC
The Dobrica's solution does not seem to work for this issue.
I have the same double login issue.
Comment 5 Jonathan Druart 2015-04-13 08:37:27 UTC
So, switching my patch back in the needs signof queue.
Comment 6 Magnus Enger 2015-04-13 09:12:39 UTC
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.
Comment 7 Magnus Enger 2015-04-13 09:19:54 UTC
I should have said "then go to the webinstaller under Plack (port 5001)"
Comment 8 Jonathan Druart 2015-04-13 09:23:07 UTC
Magnuse is running perl 5.14.

I confirm the issue on current master, using perl 5.20
Comment 9 Gaetan Boisson 2015-04-24 09:41:54 UTC
This also seems to work properly with perl 5.18
Comment 10 M. Tompsett 2015-06-14 18:11:31 UTC
(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.
Comment 11 Tomás Cohen Arazi 2015-09-07 13:42:56 UTC
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>
Comment 12 Tomás Cohen Arazi 2015-10-09 15:04:22 UTC
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.
Comment 13 Tomás Cohen Arazi 2015-10-09 17:34:09 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 14 Jonathan Druart 2015-12-09 15:13:37 UTC
This fix is not good for the performance.

I have reverted it and cannot longer reproduce the original bug.
Could someone confirm?
Comment 15 Jacek Ablewicz 2015-12-10 11:24:14 UTC
(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).
Comment 16 Katrin Fischer 2016-02-15 22:49:26 UTC
Do we need to take another look at this?
Comment 17 Jonathan Druart 2016-02-16 08:51:20 UTC
(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?
Comment 18 Jonathan Druart 2016-03-03 16:47:18 UTC
See bug 15973 for the revert.