Bug 23191

Summary: Administrators should be able to install plugins from the command line
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: Plugin architectureAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: alex.arnaud, arthur.suzuki, dcook, fridolin.somers, jonathan.druart, katrin.fischer, kyle, mtompset, robin, tomascohen
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25671
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25672
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This patch adds a new script `misc/devel/install_plugins.pl ` which allows system administrators the option of installing plugins via the command line as opposed to requiring the web side UI.
Version(s) released in:
19.11.00
Bug Depends on: 21073    
Bug Blocks: 25549    
Attachments: Bug 23191: Add commandline script to install plugins
Bug 23191: Add commandline script to install plugins
Bug 23191: Add commandline script to install plugins
Bug 23191: (follow-up) Improve output
Bug 23191: (follow-up) Improve output
Bug 23191: Add commandline script to install plugins
Bug 23191: (follow-up) Improve output

Description Martin Renvoize 2019-06-24 11:10:00 UTC
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.
Comment 1 Martin Renvoize 2019-06-24 12:34:12 UTC
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.
Comment 2 Martin Renvoize 2019-06-24 12:40:33 UTC
s/now/not in description.. man I wish i could go back and fix bugzilla descriptions/comments sometimes.
Comment 3 Alex Arnaud 2019-06-24 13:06:32 UTC
Script install already installed plugins. Means that the install method is called again. Not safe IMO.
Comment 4 Mark Tompsett 2019-06-24 13:16:21 UTC
(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.
Comment 5 Alex Arnaud 2019-06-24 14:02:23 UTC
(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.
Comment 6 Alex Arnaud 2019-06-24 14:03:49 UTC
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>
Comment 7 Martin Renvoize 2019-06-24 17:00:35 UTC
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.
Comment 8 Martin Renvoize 2019-06-24 17:01:29 UTC
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.
Comment 9 Katrin Fischer 2019-06-25 05:55:55 UTC
I think reinstalling might be a real issue: What if the plugin comes with its own data, will it get killed?
Comment 10 Katrin Fischer 2019-06-25 05:58:15 UTC
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?
Comment 11 Martin Renvoize 2019-06-25 06:47:08 UTC
(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 :(
Comment 12 Martin Renvoize 2019-06-25 06:47:59 UTC
I suppose I could call 'GetPlugins' before 'InstallPlugins' and only output the differential list..  would that be preferred?
Comment 13 Alex Arnaud 2019-06-25 07:36:18 UTC
(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.
Comment 14 Alex Arnaud 2019-06-25 07:37:00 UTC
(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*
Comment 15 Martin Renvoize 2019-06-25 08:23:13 UTC
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>
Comment 16 Martin Renvoize 2019-06-25 08:23:16 UTC
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.
Comment 17 Martin Renvoize 2019-06-25 08:27:36 UTC
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.
Comment 18 Martin Renvoize 2019-06-25 08:30:03 UTC
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'
Comment 19 Kyle M Hall 2019-06-25 11:46:00 UTC
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>
Comment 20 Kyle M Hall 2019-06-25 11:46:19 UTC
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>
Comment 21 Martin Renvoize 2019-06-25 16:20:35 UTC
Nice work!

Pushed to master for 19.11.00
Comment 22 David Cook 2020-04-28 00:59:55 UTC
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>".
Comment 23 Martin Renvoize 2020-06-03 09:00:16 UTC
(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 ;)
Comment 24 David Cook 2020-06-03 23:39:47 UTC
(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.)
Comment 25 David Cook 2020-06-04 00:16:42 UTC
(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)