[Bug 28655] New: new ideas/proposal to enhance pluggability
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 Bug ID: 28655 Summary: new ideas/proposal to enhance pluggability Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Plugin architecture Assignee: koha-bugs@lists.koha-community.org Reporter: koha@trust-box.at QA Contact: testopia@bugs.koha-community.org see discussion to 28026 -- 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=28655 --- Comment #1 from Mark Hofstetter <koha@trust-box.at> --- Created attachment 122542 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122542&action=edit see 28026 THIS IS NOT A BUGFIX JUST A PROVE OF CONCEPT just run t/patron-object-hook.t and see what happens to make this work properly something like https://metacpan.org/pod/Module::Pluggable should be used to find & include the plugins Signed-off-by: Mark Hofstetter <mark@hofstetter.at> https://bugs.koha-community.org/show_bug.cgi?id=28026 -- 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=28655 Mark Hofstetter <koha@trust-box.at> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 --- Comment #2 from Mark Hofstetter <koha@trust-box.at> --- Created attachment 122574 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122574&action=edit use Module-Pluggable as Plugin infrastructure -- 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=28655 Mark Hofstetter <koha@trust-box.at> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- I was just looking at Hook::LexWrap, and it looks like it hasn't been updated since 2017 and the two most recent updates before that were in 2014 and 2010. I suppose code like this wouldn't require many changes, but I wonder how well it is maintained. I also like how the perldoc says "There are undoubtedly serious bugs lurking somewhere in code this funky :-)". That said, it does look like Debian packages it: https://packages.debian.org/buster/libhook-lexwrap-perl I have some other concerns about the approach here, but I'm going to check them in a few minutes. -- 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=28655 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- Your test plan should be more comprehensive: 1. apt-get install libhook-lexwrap-perl 2. perl t/patron-object-hook.t -- 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=28655 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 122574 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122574 use Module-Pluggable as Plugin infrastructure Review of attachment 122574: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=28655&attachment=122574) ----------------------------------------------------------------- ::: Koha/Object.pm @@ +110,5 @@
+ my $ok = Koha::ObjectHooks->new(); + my @plugins = $ok->plugins(); + foreach my $plugin (@plugins) { + $plugin->run(); + }
Running this code in _new_from_dbic() isn't scalable. If you fetched 1000 biblios, you'd be running this code 1000 times (at least). Since you're looking to wrap subs, it would be more performant to do this at compile time ran than run time (although that would mean it would be less dynamic but dynamic plugins are problematic anyway in a persistent (eg Plack) context). -- Also, this code won't work in all contexts. If Koha::Patron hasn't been loaded into memory, you'll get the following fatal error: "Can't wrap non-existent subroutine Koha::Patron::fixup_cardnumber". It works in your example, but it has a race condition which would break in unexpected ways. That said, the way around that would be to make your plugins specific to the Koha::Object you're trying to use, so that when you load Koha::Patron, you're just fetching plugins for 'Koha::Patron' so you can always count on it being loaded. But probably solvable. -- 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=28655 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- I admit that I'm intrigued by the overall premise though. I like the idea of being able to upload plugins which can implement hooks that haven't previously been defined. That said... there could be some security implications. I mean... the whole plugin infrastructure at the moment is a security problem. But allowing a plugin to arbitrarily wrap functions could let a plugin completely take over Koha in a rather insidious way which could be difficult to detect. Of course, now I'm dreaming up potential exploits, which takes me back to bug 24632 to build a trust system so that the likelihood of running malicious code is reduced... -- 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=28655 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- (Apologies if I sound too critical. I'm not intending to be discouraging. I'm trying to provide helpful feedback. I would like to see more on this topic.) -- 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=28655 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=28655 Mark Hofstetter <koha@trust-box.at> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |koha@trust-box.at |ity.org | CC| |tomascohen@gmail.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=28655 Mark Hofstetter <koha@trust-box.at> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 Mark Hofstetter <koha@trust-box.at> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 Mark Hofstetter <koha@trust-box.at> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=34713 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 --- Comment #8 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- I'm very interested in this and would love to see Koha moving in this direction. I don't like that every new niche little requirement needs to be pushed to core. Mark, you were working on this during last HackFest 2023? Do you have updates on this? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I not opposed to this approach so long as we can resolve the issues already noted by David. I would really like to see more formal trust process for security with a more centralised app store style approach.. I see that as a prerequisite for this level of exposure/power in the plugin system. I also feel we'd need to massively improve our code documentation.. right now we make no promises to keep subroutine signatures or names etc as they are.. in fact with all the refactoring and modernisation work going on we're changing things under the hood a lot. Opening up every method to being wrapped opens up a new can if worms and requirements for core to become significantly more stable than I think it currently is. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28173 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 Thomas Klausner <domm@plix.at> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |domm@plix.at -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jacob.omara@ptfs-europe.com | |, | |matt.blenkinsop@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=28655 --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Martin Renvoize from comment #9)
I not opposed to this approach so long as we can resolve the issues already noted by David.
I would really like to see more formal trust process for security with a more centralised app store style approach.. I see that as a prerequisite for this level of exposure/power in the plugin system.
I also feel we'd need to massively improve our code documentation.. right now we make no promises to keep subroutine signatures or names etc as they are.. in fact with all the refactoring and modernisation work going on we're changing things under the hood a lot. Opening up every method to being wrapped opens up a new can if worms and requirements for core to become significantly more stable than I think it currently is.
Agreed! I can see us versioning our signatures so to make it easier to know if a plugin may no longer be compatible. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|new ideas/proposal to |New ideas/proposal to |enhance pluggability |enhance pluggability -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28655 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |arthur.suzuki@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org