Some support companies would prefer to now allow end users to install new plugins. Prior to bug 21073 this was mostly possible by enabling plugins in the config but making the plugin path read only to the apache user. In this way users could not upload new plugins, but support providers could add plugins at the users request. This did have the caveat that 'install', 'update' and 'remove' routines in the plugin would not be called. With bug 21073 we must run through the InstallPlugins method to populate the relevant database tables so we should add a command line script that would allow administrators to run this once they had manually added plugin code to the correct paths.
Created attachment 90943 [details] [review] Bug 23191: Add commandline script to install plugins This adds an initial skeliton script to enable plugin installation at the commandline.
s/now/not in description.. man I wish i could go back and fix bugzilla descriptions/comments sometimes.
Script install already installed plugins. Means that the install method is called again. Not safe IMO.
(In reply to Alex Arnaud from comment #3) > Script install already installed plugins. Means that the install method is > called again. Not safe IMO. In the InstallPlugins method there is this line of code: Koha::Plugins::Methods->search({ plugin_class => $plugin_class })->delete(); You can call InstallPlugins as many times as you want, because it is reinstall the second time.
(In reply to M. Tompsett from comment #4) > (In reply to Alex Arnaud from comment #3) > > Script install already installed plugins. Means that the install method is > > called again. Not safe IMO. > > In the InstallPlugins method there is this line of code: > Koha::Plugins::Methods->search({ plugin_class => $plugin_class > })->delete(); > > You can call InstallPlugins as many times as you want, because it is > reinstall the second time. Good! I was worried about script output, but i seems that InstallPlugins only reset plugin methods.
Created attachment 90946 [details] [review] Bug 23191: Add commandline script to install plugins This adds an initial skeliton script to enable plugin installation at the commandline. Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Yeah, I'm not the biggest fan of the fact that InstallPlugins re-installs all plugins every run.. but it is indeed idempotent in the way it does so. I argued for a cleaner approach in the performance bug but we decided to push forward and refine that code as a next step. You are right though, I thought the output was a nice extra to show 'something' had happened, but in this case it may be more confusing than helpful.. totally open to some code gulf or suggestions.
Also, it's not actually a change.. the code before the performance bug also iterated through all plugins each time install was called.. it was just less obvious about it.
I think reinstalling might be a real issue: What if the plugin comes with its own data, will it get killed?
I am not sure how things work really. Just wondering: if you had a plugin installed already that has added data, will reinstalling effect it?
(In reply to Katrin Fischer from comment #9) > I think reinstalling might be a real issue: What if the plugin comes with > its own data, will it get killed? So the re-installing all really isn't a change.. that's always happened.. for any plugin you installed we would iterate and re-install all other plugins at the same time. By 'install' I mean literally telling Koha that the plugin exists and what methods are available within it.. we update the plugin_methods table but we do not update any internal plugin data, that all remains intact. We will trigger the 'install' or 'upgrade' methods from within the plugin if required (i.e. we check versions within the database and only run the install if it has not been run before, or the upgrade if we've caught a version change). It is up to the plugin author to make sure their 'upgrade' method doesn't splat older data during such actions. In short.. I think the re-install loop is a non-issue.. it's a minor annoyance performance wise but it shouldn't cause any harm and in reality is unrelated to this patch.. Clearly the fact that I output the actual results of the script run is confusing people :(
I suppose I could call 'GetPlugins' before 'InstallPlugins' and only output the differential list.. would that be preferred?
(In reply to Martin Renvoize from comment #12) > I suppose I could call 'GetPlugins' before 'InstallPlugins' and only output > the differential list.. would that be preferred? May be it could be less confusion indeed. Even if InstallPlugins doesn't call plugin's install methods.
(In reply to Alex Arnaud from comment #13) > (In reply to Martin Renvoize from comment #12) > > I suppose I could call 'GetPlugins' before 'InstallPlugins' and only output > > the differential list.. would that be preferred? > > May be it could be less confusion indeed. Even if InstallPlugins doesn't > call plugin's install methods. Confusing*
Created attachment 90979 [details] [review] Bug 23191: Add commandline script to install plugins This adds an initial skeliton script to enable plugin installation at the commandline. Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Created attachment 90980 [details] [review] Bug 23191: (follow-up) Improve output This patch adds a check of what plugins installed before the script run and outputs only those plugins that have been installed for the first time or upgraded during this run.
Created attachment 90981 [details] [review] Bug 23191: (follow-up) Improve output This patch adds a check of what plugins installed before the script run and outputs only those plugins that have been installed for the first time or upgraded during this run.
New test plan: 1) Without any plugins installed, run the script. Output should be: No plugins found at $plugins_dir 2) Manually add an unzipped plugin into your plugins_dir 3) Run the script. Output should be: 'Installed $plugin_name $plugin_version' 4) Run the script again. Output should be: 'All plugins successfully re-initialised' 5) Increment the 'version' in your unpacked plugin. 6) Run the script. Output should be: 'Upgrade $plugin_name from version $old_version to version $new_version'
Created attachment 90985 [details] [review] Bug 23191: Add commandline script to install plugins This adds an initial skeliton script to enable plugin installation at the commandline. Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 90986 [details] [review] Bug 23191: (follow-up) Improve output This patch adds a check of what plugins installed before the script run and outputs only those plugins that have been installed for the first time or upgraded during this run. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Nice work! Pushed to master for 19.11.00
I like the sound of this but the implementation seems very different to what I would have expected. I thought it would be something like "misc/devel/install_plugin.pl <path/to/kpz>".
(In reply to David Cook from comment #22) > I like the sound of this but the implementation seems very different to what > I would have expected. > > I thought it would be something like "misc/devel/install_plugin.pl > <path/to/kpz>". Feel free to add such this additional functionality in a new bug ;)
(In reply to Martin Renvoize from comment #23) > (In reply to David Cook from comment #22) > > I like the sound of this but the implementation seems very different to what > > I would have expected. > > > > I thought it would be something like "misc/devel/install_plugin.pl > > <path/to/kpz>". > > Feel free to add such this additional functionality in a new bug ;) It's on my mind! Heh. I just have so much going on at the moment. I haven't even been working on plugins, as I've just needed the free time to decompress. (I'm just about to roll out some experimental work with RabbitMQ to select production instances, so I might have more to say on Bug 22417 at least.)
(In reply to Martin Renvoize from comment #23) > (In reply to David Cook from comment #22) > > I like the sound of this but the implementation seems very different to what > > I would have expected. > > > > I thought it would be something like "misc/devel/install_plugin.pl > > <path/to/kpz>". > > Feel free to add such this additional functionality in a new bug ;) I'm thinking "koha-plugin-install <instance> <plugin>". Then if you wanted all your instances to be running one plugin, it would be trivial... Ok I'll open a bug report now but probably won't work on for a while (although you never know :O)