[Bug 30897] New: Gracefully reload Koha after plugin install/upgrade
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Bug ID: 30897 Summary: Gracefully reload Koha after plugin install/upgrade Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Plugin architecture Assignee: koha-bugs@lists.koha-community.org Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org When a plugin is installed/upgraded, it would be good to send a SIGHUP to the Starman master, so that Koha will gracefully restart itself. As Kyle noted on the listserv, it is especially useful for plugins that add API routes, since the API can't use those until Starman workers are restarted. By doing a graceful restart, no active HTTP requests are interrupted. -- 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=30897 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21366 -- 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=30897 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com | |, kyle@bywatersolutions.com --- Comment #1 from David Cook <dcook@prosentient.com.au> --- We should be able to just run: kill('HUP',$starman_master_pid); However, the tough part is getting $starman_master_pid. As a human, I know the PID for kohadev is stored at /var/run/koha/kohadev/plack.pid but Koha itself doesn't know that. The "koha-plack" script defines the PIDFILE like so: local PIDFILE="/var/run/koha/${instancename}/plack.pid" -- I suppose another option might be to invoke "koha-plack --reload $instance_name" (after bug 21366), but that would assume that it's a Debian package installation and that we have the $instance_name, which we also don't have. -- I suppose the other thing is that this would be Starman specific. I don't know how it would work with Hypnotoad for instance. Based on https://metacpan.org/pod/Mojo::Server::Hypnotoad#MANAGER-SIGNALS it looks like it's graceful restart is done via the USER2 signal rather than the HUP signal. -- 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=30897 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |DUPLICATE --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** This bug has been marked as a duplicate of bug 31074 *** -- 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=30897 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 164587 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164587&action=edit Bug 30897: Gracefully restart plack after plugin install/remove -- 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=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com CC| |martin.renvoize@ptfs-europe | |.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=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |critical -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=31074 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|DUPLICATE |--- Status|RESOLVED |REOPENED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164587|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 164588 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164588&action=edit Bug 30897: Gracefully restart plack after plugin install/remove -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #5 from David Cook <dcook@prosentient.com.au> --- Thanks for looking into this, Martin. I had forgotten that I'd raised this issue! I'll have to mark it as Failed QA for a few reasons. 1a. If you run install_plugins.pl from the CLI, your bash session will be killed, since that's the parent process. I think that's an unintended consequence. (However, we can probably work around this by checking if we're running in a Plack environment first.) root@kohadevbox:koha$ koha-shell kohadev $ /kohadevbox/koha/misc/devel/install_plugins.pl No plugins found at /var/lib/koha/kohadev/plugins Hangup root@kohadevbox:koha$ 1b. Like 1a, this will cause problems when running unit tests: kohadev-koha@kohadevbox:koha(bug30897)$ perl t/db_dependent/Koha/Plugins/Circulation_hooks.t 1..4 ok 1 - use Koha::Plugins; ok 2 - use Koha::Plugins::Handler; ok 3 - use Koha::Plugin::Test; # Subtest: after_circ_action() hook tests 1..3 Hangup root@kohadevbox:koha$ 2. I think we should wrap this with some configuration. Restarting the Starman on plugin installation is a significant change, so I think we'll want to be able to quickly and easily change between behaviours - at least in the short-term. I think we'll also probably want to default to off at least initially just to be on the safe side. 3. This restart runs even if there are no changes to make to the plugins, which would lead to unnecessary restarts. I think we'd want to be more careful in initiating the restarts. -- Additional questions/comments: 1. Could you speak to why you need to look for the parent pid recursively? I don't see the reason for it. As an aside, do we want to use POSIX::getpid instead of $$ for getting the pid? I suppose POSIX::getpid is easier to read. 2. Relating to my first failure point, do you think perhaps we should interrogate the process we're looking to restart just to be extra careful? At a minimum, I think we'd want to double-check that it is really Starman. I wanted to say we shouldn't try to "kill" pid 1, but I suppose if someone has managed to get Starman running in Docker as the only process, then it could have a pid of 1, and that would be OK... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 164654 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164654&action=edit Bug 30897: Limit to plack environments I had a feeling we'd want to do this when I submitted the proof of concept, though I hadn't appreciated the wider issues that have now been highlighted.. it makes a lot of sense to limit to the plack scope. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for your review David :). I had a feeling we'd want to wrap it to check we were running as plack but then totally forgot to add it.. follow-up added. Though we might also want to somehow try to let this happen for background workers and I'm not entirely sure how to identify those yet (I think we can probably handle that on another bug). As for the recursive lookup, I must put my hands up here and say I followed some google example from somewhere.. we may well only need to get the single parent.. I can't see that we allow children to spawn more children anywhere here in plack so we're probably safe to reduce that code. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- As for $$ vs POSIX... I honestly had just forgotten that $$/$PID/$PROCESS_ID existed.. having said that I feel like the POSIX import makes it very clear what we're doing... I could see devs not immediately understanding the use of $$. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Do you have a method in mind for the double check we're starman/plack you mention? I'm struggling to think of a reliable one -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize from comment #9)
Do you have a method in mind for the double check we're starman/plack you mention?
I'm struggling to think of a reliable one
I was hoping that we could check $0 but it looks like one of the Plack components possibly relating to Plack::App::CGIBin changes it to the CGI script being emulated, so that won't work. I think we'll probably just have to check the environment in that case. C4::Context->psgi_env() will probably be good enough. Another thought about this one... rather than putting it into the InstallPlugins(), we could have a different method for resetting the parent process, and just invoke it in the web controller. That would be safer. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize from comment #8)
As for $$ vs POSIX... I honestly had just forgotten that $$/$PID/$PROCESS_ID existed.. having said that I feel like the POSIX import makes it very clear what we're doing... I could see devs not immediately understanding the use of $$.
Yeah, I think so too. While Perl shorthand can be handy, I do like clarity more. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #12 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize from comment #7)
I had a feeling we'd want to wrap it to check we were running as plack but then totally forgot to add it.. follow-up added. Though we might also want to somehow try to let this happen for background workers and I'm not entirely sure how to identify those yet (I think we can probably handle that on another bug).
Mmm that's a good point. I bumped into that issue myself recently. I can't remember what it was exactly but yeah the background workers didn't get the update because I didn't think to restart them...
As for the recursive lookup, I must put my hands up here and say I followed some google example from somewhere.. we may well only need to get the single parent.. I can't see that we allow children to spawn more children anywhere here in plack so we're probably safe to reduce that code.
Cool I agree -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #10)
I think we'll probably just have to check the environment in that case. C4::Context->psgi_env() will probably be good enough.
I just noticed that you already did this in your new patch heh. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164588|0 |1 is obsolete| | --- Comment #14 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 164655 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164655&action=edit Bug 30897: Gracefully restart plack after plugin install/remove == test plan == 1. list your worker processes with something like htop 2. install a plugin https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/releases 3. note that the PIDs of the workers stay the same and no sign of restart (no PID changes and no CPU activity that the workers would have if they were starting) 4. apply patches and restart services 5. unistall plugin 6. notice worker restart 7. install plugin 8. notice worker restart 9. downgrade plugin 10. notice worker restart 11. upgrade plugin 12. notice worker restart 13. disable and enable plugin 14. *no restart* Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #15 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Oops, mid air collision. Thanks David for finding these issue. So I removed my signoff line. And kept the test plan that I added in the commit message. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164655|0 |1 is obsolete| | --- Comment #16 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 164657 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164657&action=edit Bug 30897: Gracefully restart plack after plugin install/remove == test plan == 1. list your worker processes with something like htop 2. install a plugin https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/releases 3. note that the PIDs of the workers stay the same and no sign of restart (no PID changes and no CPU activity that the workers would have if they were starting) 4. apply patches and restart services 5. unistall plugin 6. notice worker restart 7. install plugin 8. notice worker restart 9. downgrade plugin 10. notice worker restart 11. upgrade plugin 12. notice worker restart 13. disable and enable plugin 14. *no restart* -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164654|0 |1 is obsolete| | --- Comment #17 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 164658 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164658&action=edit Bug 30897: Limit to plack environments I had a feeling we'd want to do this when I submitted the proof of concept, though I hadn't appreciated the wider issues that have now been highlighted.. it makes a lot of sense to limit to the plack scope. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #18 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Martin Renvoize from comment #7)
Though we might also want to somehow try to let this happen for background workers and I'm not entirely sure how to identify those yet (I think we can probably handle that on another bug).
What kind of issues would a background worker have ? Adding API routes: N/A for background workers. But I suspect that bug 31074 comment 0 would have implication to background workers. +1 for as follow-up ticket. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #19 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Martin Renvoize from comment #7)
As for the recursive lookup, I must put my hands up here and say I followed some google example from somewhere.. we may well only need to get the single parent.. I can't see that we allow children to spawn more children anywhere here in plack so we're probably safe to reduce that code.
Ok, will keep an eye when it's ready to be tested again. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164657|0 |1 is obsolete| | --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 164660 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164660&action=edit Bug 30897: Gracefully restart plack after plugin install/remove This patch adds a graceful plack restart to the plugin install/uninstall routine to fully allow plugin install to work via the staff client. Test plan 1. list your worker processes with something like htop 2. install a plugin https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/releases 3. note that the PIDs of the workers stay the same and no sign of restart (no PID changes and no CPU activity that the workers would have if they were starting) 4. apply patches and restart services 5. unistall plugin 6. notice worker restart 7. install plugin 8. notice worker restart 9. downgrade plugin 10. notice worker restart 11. upgrade plugin 12. notice worker restart 13. disable and enable plugin 14. *no restart* Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164658|0 |1 is obsolete| | --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 164661 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164661&action=edit Bug 30897: Limit to plack environments I had a feeling we'd want to do this when I submitted the proof of concept, though I hadn't appreciated the wider issues that have now been highlighted.. it makes a lot of sense to limit to the plack scope. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 164662 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164662&action=edit Bug 30897: Add option to disable automated restart This patch adds the ability to disable the automated plack restart we introduce with this patchset via configuration. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 164663 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164663&action=edit Bug 30897: Remove recursive parent identification loop We believe that plack will only spawn from one parent and children cannot spawn further child processes.. as such we don't need to walk up the tree. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Whilst it's tempting to squash some of these I think the commit notes and history are useful to future devs here. I think I've resolved all the worries here and set the restart to default disabled (I'd love to default to on but understand the concerns with that) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- *** Bug 31074 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jake.deery@ptfs-europe.com, | |liz@bywatersolutions.com, | |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164660|0 |1 is obsolete| | Attachment #164661|0 |1 is obsolete| | Attachment #164662|0 |1 is obsolete| | Attachment #164663|0 |1 is obsolete| | --- Comment #26 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 164735 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164735&action=edit Bug 30897: Gracefully restart plack after plugin install/remove This patch adds a graceful plack restart to the plugin install/uninstall routine to fully allow plugin install to work via the staff client. Test plan 1. list your worker processes with something like htop 2. install a plugin https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/releases 3. note that the PIDs of the workers stay the same and no sign of restart (no PID changes and no CPU activity that the workers would have if they were starting) 4. apply patches and restart services 5. unistall plugin 6. notice worker restart 7. install plugin 8. notice worker restart 9. downgrade plugin 10. notice worker restart 11. upgrade plugin 12. notice worker restart 13. disable and enable plugin 14. *no restart* Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #27 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 164736 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164736&action=edit Bug 30897: Limit to plack environments I had a feeling we'd want to do this when I submitted the proof of concept, though I hadn't appreciated the wider issues that have now been highlighted.. it makes a lot of sense to limit to the plack scope. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #28 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 164737 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164737&action=edit Bug 30897: Add option to disable automated restart This patch adds the ability to disable the automated plack restart we introduce with this patchset via configuration. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #29 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 164738 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164738&action=edit Bug 30897: Remove recursive parent identification loop We believe that plack will only spawn from one parent and children cannot spawn further child processes.. as such we don't need to walk up the tree. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |nick@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #30 from Nick Clemens <nick@bywatersolutions.com> --- This works well, starts out disabled, nice to see API routes show quickly, excellent! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #31 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 164758 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164758&action=edit Bug 30897: (QA follow-up) Enable graceful restart by default This patch enabled the restart by default. After a poll at hackfest24 we opted to enable this by default and the RM requested I add the patch to the bug so we don't forget ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #32 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #33 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 164737 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164737 Bug 30897: Add option to disable automated restart Review of attachment 164737: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=30897&attachment=164737) ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-upload.tt @@ +45,5 @@
</div> [% END %] + [% IF ( !plugins_restart ) %] + <div class="dialog alert"> + <strong>You're system is not configured to automatically refresh on plugin upload, you may need to ask your administrator to complete the plugin installation.</strong>
I notice the patch is spelled "You're system" but the patch in master is spelled correctly as "Your system". We've still got a comma splice here, so that comma should be replaced with either a full stop or a semicolon. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #34 from David Cook <dcook@prosentient.com.au> --- Also, one final concern: For DB version 19.06.00.006, we run "Koha::Plugins->new({ enable_plugins => 1 })->InstallPlugins;" in installer/data/mysql/updatedatabase.pl In theory, if the database were being upgraded via the web installer, this would cause the database upgrade to crash I think? I might just test that quickly... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #35 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #34)
Also, one final concern:
I might just test that quickly...
So the Starman worker launches a subprocess like the following: sh -c /kohadevbox/koha/installer/data/mysql/updatedatabase.pl >> /var/log/koha/kohadev/updatedatabase_2024-04-12T02:34:37_19.0600004_23.1200017.log 2>> /var/log/koha/kohadev/updatedatabase- error_2024-04-12T02:34:37_19.0600004_23.1200017.log So it's this shell process that gets HUPed, and then init takes over as the parent process: kohadev-koha@kohadevbox:koha(bug_30897_1)$ ps -fww -p 15743 UID PID PPID C STIME TTY TIME CMD kohadev+ 15743 1 13 02:32 ? 00:00:01 /usr/bin/perl /kohadevbox/koha/installer/data/mysql/updatedatabase.pl -- I think this is another unintended consequence, but... it probably won't cause any harm overall, and it would only affect people upgrading from <= 19.05 to >= 24.05. (Note that the upgrade crashed for me at 19.0600011 but that was for unrelated reasons.) -- Note that this is one of the reasons why I think the Starman restart should've been in a separate function to InstallPlugins(). While the psgi detection helped, it's not quite enough. In this case, the environmental variables are inherited from the web process by the shell process. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #36 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Martin Renvoize from comment #31)
After a poll at hackfest24 we opted to enable this by default and the RM requested I add the patch to the bug so we don't forget ;)
+1 it's better to have good default for libraries without a sysadmin or support provider (the more likely to use the plugin upload UI without knowing about the shortcomings) to have correct behaving plugin management. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #37 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Nice find David about the installer, we are lucky it's not that bad. --- Thanks all for managing to squash this issue! :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This fix, if enabled, release notes| |allows plugins to be | |installed via the staff | |client without the | |additional need to ask a | |system administrator to | |restart your Koha instance. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|critical |enhancement -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This fix, if enabled, |This fix, if enabled, release notes|allows plugins to be |allows plugins to be |installed via the staff |installed via the staff |client without the |interface without the |additional need to ask a |additional need to ask a |system administrator to |system administrator to |restart your Koha instance. |restart your Koha instance. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This fix, if enabled, |This fix, (enabled by release notes|allows plugins to be |default for new |installed via the staff |installations), allows |interface without the |plugins to be installed via |additional need to ask a |the staff interface without |system administrator to |the additional need to ask |restart your Koha instance. |a system administrator to | |restart your Koha instance. --- Comment #38 from Victor Grousset/tuxayo <victor@tuxayo.net> ---
After a poll at hackfest24 we opted to enable this by default
Release notes updated. It's for new installs, right? IIUC the absence of <plugins_restart> is the same as it being = 0 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #39 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Correct and correct, thanks Victor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #40 from Fridolin Somers <fridolin.somers@biblibre.com> --- Not backported to 23.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wizzyrea@gmail.com --- Comment #41 from Liz Rea <wizzyrea@gmail.com> --- This feature seems to break searching to install plugins, nothing happens when we try to search for a plugin and click "install" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 --- Comment #42 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Liz Rea from comment #41)
This feature seems to break searching to install plugins, nothing happens when we try to search for a plugin and click "install"
Looks like it's actually another CSRF regression.. I'll open a new bug and put the fix there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30897 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=37916 --- Comment #43 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- See bug 37916 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org