Bug 23191 - Administrators should be able to install plugins from the command line
Summary: Administrators should be able to install plugins from the command line
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords: Manual
Depends on: 21073
Blocks:
  Show dependency treegraph
 
Reported: 2019-06-24 11:10 UTC by Martin Renvoize
Modified: 2019-11-14 11:42 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
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


Attachments
Bug 23191: Add commandline script to install plugins (2.68 KB, patch)
2019-06-24 12:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23191: Add commandline script to install plugins (2.73 KB, patch)
2019-06-24 14:03 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 23191: Add commandline script to install plugins (2.73 KB, patch)
2019-06-25 08:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23191: (follow-up) Improve output (2.28 KB, patch)
2019-06-25 08:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23191: (follow-up) Improve output (2.40 KB, patch)
2019-06-25 08:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23191: Add commandline script to install plugins (2.80 KB, patch)
2019-06-25 11:46 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23191: (follow-up) Improve output (2.47 KB, patch)
2019-06-25 11:46 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 M. 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