[Bug 36736] New: Add ability to load DBIx::Class Schema files found in plugins
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Bug ID: 36736 Summary: Add ability to load DBIx::Class Schema files found in plugins Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Right now, it is not possible to create custom Koha::Object(s) for a plugin without breaking things. For example we did this: BEGIN { my $path = Module::Metadata->find_module_by_name(__PACKAGE__); $path =~ s!\.pm$!/lib!; unshift @INC, $path; require Koha::CurbsidePickupIssues; require Koha::CurbsidePickupPolicies; require Koha::CurbsidePickups; require Koha::Schema::Result::CurbsidePickup; require Koha::Schema::Result::CurbsidePickupIssue; require Koha::Schema::Result::CurbsidePickupPolicy; # register the additional schema classes Koha::Schema->register_class(CurbsidePickup => 'Koha::Schema::Result::CurbsidePickup'); Koha::Schema->register_class(CurbsidePickupPolicy => 'Koha::Schema::Result::CurbsidePickupPolicy'); Koha::Schema->register_class(CurbsidePickupIssue => 'Koha::Schema::Result::CurbsidePickupIssue'); # ... and force a refresh of the database handle so that it includes # the new classes Koha::Database->schema({ new => 1 }); } In the curbside pickups plugin before it was integrated. The problem we've found is that calling Koha::Database->schema({ new => 1 }); resets the database connection and breaks any transactions you were in when the plugin was loaded! We should be able to search for and load any schema files found in plugin directories to make this all unnecessary. -- 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=36736 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Forgot to mention, we probably need to do that search and load in Koha::Schema -- 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=36736 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |blawlor@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #2 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- So, I played around here a bit - my first thought was that we could simply avoid the reconnect if the objects were already loaded, so I updated a plugin like so: $schema = Koha::Database->schema({ new => 1 }) unless $schema->source_registrations->{KohaPluginComBywatersolutionsContractsContract}; With some warns I found that this was executed when plack was restarted, so the object are loaded in plack from the start, that's good. But I found that imports still failed and caused reconnect - that's bad. What I eventually realized was that we are forking our background jobs - and these get a new DB connection, without the new objects. The dirty fix that worked was adding to the worker: Koha::Plugins->get_enabled_plugins; After spawning the process, but before beginning the job. And that fixes it. I am not sure where we load the plugins when we start plack, but if we did the same thing when launching a background worker process - or built it in to the process method of BackgroundJob To move the logic out of the plugin we could add a 'plugin_classes' method and have Koha::Schema check for if the objects are loaded - but I think maybe just loading plugins for the background tasks at spawn is a good start? -- 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=36736 --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- That sounds like a reasonable plan to me! -- 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=36736 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=36736 --- Comment #4 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 168861 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=168861&action=edit Bug 36736: Load plugins at the start of background job processing This patch adds a call to get_enabled_plugins before processing background jobs to ensure that all plugin hooks are loaded and cached To test: 1 - Install a plugin that adds new objects e.g. the Contracts plugin https://github.com/bywatersolutions/fs-koha-plugin-contracts or the KOha Advent plugin: https://gitlab.com/koha-community/koha-advent/koha-plugin-fancyplugin 2 - Restart all 3 - Tail all your logs 4 - Stage and import a file containing items 5 - Note in the logs DBI Exception: DBD::mysql::st execute failed: Lock wait timeout exceeded; try restarting transaction 6 - Apply this patch 7 - Restart all 8 - Stage and import again 9 - Success! -- 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=36736 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- 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=36736 Nick Clemens (kidclamp) <nick@bywatersolutions.com> 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=36736 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- Sounds interesting. Could you walk me/us through this a bit more? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #6 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to David Cook from comment #5)
Sounds interesting. Could you walk me/us through this a bit more?
The idea is really to make plugin code a bit simpler, easier to read, more consistent with Koha core, and leave open a path for future integration. Adding new objects via the plugin lets plugins use KohaTables and other helper code as well. Adding objects was featured in the Koha Advent plugin, but it seems the background jobs broke this trick This patch just ensures the plugins are loaded, and hence all functions and objects available, for our background processes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- (In reply to Nick Clemens (kidclamp) from comment #6)
Adding new objects via the plugin lets plugins use KohaTables and other helper code as well.
I certainly like the sound of that!
Adding objects was featured in the Koha Advent plugin, but it seems the background jobs broke this trick
This patch just ensures the plugins are loaded, and hence all functions and objects available, for our background processes
So I was hoping you'd be able to explain it easier than I'd be able to puzzle it out by trying it myself, but I think it's one of those things where a person just needs to try it out. Trying the test plan, and I see the errors in /var/log/koha/kohadev/worker-output.log I'm a bit confused though... I installed the plugin, restarted everything, and we're using a plugin that isn't used in the MARC import background job. So I feel like we're not really addressing the root problem here. Actually... looking at the log /var/log/koha/kohadev/worker-output.log again I see this: Argument "{VERSION}" isn't numeric in int at /kohadevbox/koha/Koha/Plugins/Base.pm line 339. Argument "{VERSION}" isn't numeric in int at /kohadevbox/koha/Koha/Plugins/Base.pm line 339. I'm going to investigate that a bit further... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #8 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Ah, so, basically the background job looks for plugins with an 'after_biblio_action' hook. To find them we end up loading all the plugins.modules. This is what causes the begin block to fire again. If we call GetPlugins before this check, everything is cached and we don't reload the modules, so we don't trigger the new dbic -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- Okie dokie... So Koha/BackgroundJob.pm looks up any plugins by querying the database for the method "background_tasks". Excellent. That means only plugins that have the background_tasks method get loaded by the background worker when it's getting set up (prior to forking). 448 my @plugins = Koha::Plugins->new()->GetPlugins( { method => 'background_tasks', } ); -- Unfortunately... during the work of Koha/BackgroundJob/MARCImportCommitBatch.pm, we call C4::Biblio::ModBiblioMarc, which runs the following code: 2914 Koha::Plugins->call( 2915 'before_biblio_action', 2916 { action => 'save', payload => { biblio_id => $biblionumber, record => $record } } 2917 ); The "call" function loads every installed plugin and then checks if the Perl object "can" do a particular method. That causes the problem that Kyle and Nick have noted. -- We could use the patch attached here, but I'm thinking that we might want to first fix the Koha::Plugins->call() so that it uses the database lookup for the method, caches it, and goes from there. That should hopefully fix this problem *plus* improve the efficiency of Koha::Plugins->call(). I wonder if there's already a bug for that in fact... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- (In reply to Nick Clemens (kidclamp) from comment #8)
Ah, so, basically the background job looks for plugins with an 'after_biblio_action' hook. To find them we end up loading all the plugins.modules. This is what causes the begin block to fire again. If we call GetPlugins before this check, everything is cached and we don't reload the modules, so we don't trigger the new dbic
Something like that. I guess I haven't worked out the exact cause. Just that it's C4::Biblio::ModBiblioMarc loading all the modules, and that's triggering that BEGIN block on the Koha Advent plugin, which is doing that connection reset within the BatchMarcCommit transaction. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29672 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #9)
We could use the patch attached here, but I'm thinking that we might want to first fix the Koha::Plugins->call() so that it uses the database lookup for the method, caches it, and goes from there.
It looks like I talked to Julian about this years ago on bug 29672. I thought there was a newer bug... -- I was concerned about RAM usage here, but it looks like you've added the "Koha::Plugins->get_enabled_plugins();" after the background_jobs_worker.pl forks, so that's good. It would be more efficient to update Koha::Plugins->call(), but that would be more work. This is a practical solution... -- On a side note, for Koha::Plugin::FancyPlugin, does the Koha::Schema->register_class and Koha::Database->schema({ new => 1 }) have to be in the BEGIN block? Could you wrap it with a check for a plack environmental variable? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #168861|0 |1 is obsolete| | --- Comment #12 from David Cook <dcook@prosentient.com.au> --- Created attachment 169389 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169389&action=edit Bug 36736: Load plugins at the start of background job processing This patch adds a call to get_enabled_plugins before processing background jobs to ensure that all plugin hooks are loaded and cached To test: 1 - Install a plugin that adds new objects e.g. the Contracts plugin https://github.com/bywatersolutions/fs-koha-plugin-contracts or the KOha Advent plugin: https://gitlab.com/koha-community/koha-advent/koha-plugin-fancyplugin 2 - Restart all 3 - Tail all your logs 4 - Stage and import a file containing items 5 - Note in the logs DBI Exception: DBD::mysql::st execute failed: Lock wait timeout exceeded; try restarting transaction 6 - Apply this patch 7 - Restart all 8 - Stage and import again 9 - Success! Signed-off-by: David Cook <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=36736 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- If someone else adds a sign off, I think I could convert this to a Passed QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169389|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 169410 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169410&action=edit Bug 36736: Load plugins at the start of background job processing This patch adds a call to get_enabled_plugins before processing background jobs to ensure that all plugin hooks are loaded and cached To test: 1 - Install a plugin that adds new objects e.g. the Contracts plugin https://github.com/bywatersolutions/fs-koha-plugin-contracts or the Koha Advent plugin: https://gitlab.com/koha-community/koha-advent/koha-plugin-fancyplugin 2 - Restart all 3 - Tail all your logs 4 - Stage and import a file containing items 5 - Note in the logs DBI Exception: DBD::mysql::st execute failed: Lock wait timeout exceeded; try restarting transaction 6 - Apply this patch 7 - Restart all 8 - Stage and import again 9 - Success! Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Martin Renvoize <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=36736 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com 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=36736 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |ASSIGNED --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I'm writing tests for this. Hold on. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 169440 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169440&action=edit Bug 36736: Tests [POC] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Hi, my work day is ending and I wanted to share what I've got so far. I haven't managed to reproduce the error so far. I've added: * A schema file * Koha::Object-based classes for that schema * Tweaked the Test plugin so it has the usual ->register call * A plugin background job that (for now) just prints the schema it finds. I expected it to not have the schemas defined, but they are. I hope I missed something obvious and someone beats me on finishing the tests before I wake up :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #18 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #17)
Hi, my work day is ending and I wanted to share what I've got so far. I haven't managed to reproduce the error so far.
I've added: * A schema file * Koha::Object-based classes for that schema * Tweaked the Test plugin so it has the usual ->register call * A plugin background job that (for now) just prints the schema it finds.
I expected it to not have the schemas defined, but they are.
I hope I missed something obvious and someone beats me on finishing the tests before I wake up :-D
You won't be able to reproduce the problem in "t/db_dependent/Koha/Plugins/Plugins.t", because Koha::Plugin::Test is already loaded in the BEGIN block of this test file. We'll need to have a separate test file just for this test. We'll want to add a Koha::Plugins->call() within a database transaction into Koha::BackgroundJob::TestBackgroundJob->process(), because that's what triggers the bug. Koha::Plugins->call() loads all enabled plugins, which will then reset the connection, and since we're in a database transaction, then we'll get a lock issue I believe. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #19 from David Cook <dcook@prosentient.com.au> --- Let's see if I can really quickly bang this out... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #20 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #19)
Let's see if I can really quickly bang this out...
Interrupted by too many meetings. Hoping to sit down and look at this later today. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #21 from David Cook <dcook@prosentient.com.au> --- I've had a go at writing the test, but I've actually made it so that even with the patch, the problem will still occur. I think because we wrap all our tests with transactions it makes the database connection stuff problematic. But then again... I'm having all kinds of weird database issues in this test. Like I'm inserting data before the transactions... and I still can't get the following to work: my @plugins = Koha::Plugins->new()->GetPlugins( { method => 'background_tasks', } ); But I've wasted way too much time on this already... I don't think I can work on this anymore. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #22 from David Cook <dcook@prosentient.com.au> --- Btw another thing I noticed with the current attached test... in order to utilize Nick's change, you need to use create the background job via Koha::BackgroundJob and call its process() method, which then will invoke the background job type listed in its "type" attribute. Like Julian says on bug 29672, maybe we should be relying on a standardized bootstrap step instead. In any case, I think there's a lot of pain ahead with this in any case... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- It seems I forgot to add my last attempt yesterday: ``` index 460b710a4e5..58ffc9aadac 100755 --- a/t/db_dependent/Koha/Plugins/Plugins.t +++ b/t/db_dependent/Koha/Plugins/Plugins.t @@ -322,9 +322,13 @@ subtest 'Koha::Plugin::Test' => sub { plan tests => 1; - use DDP; - my $job = Koha::BackgroundJob::TestBackgroundJob->new; - $job->process; + # Launch the sleep script + my $pid = fork(); + if ( $pid == 0 ) { + use DDP; + my $job = Koha::BackgroundJob::TestBackgroundJob->new; + $job->process; + } ``` which didn't work either :-P -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169440|0 |1 is obsolete| | --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 169508 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169508&action=edit Bug 36736: Tests [POC] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #25 from Tomás Cohen Arazi <tomascohen@gmail.com> --- So it now explodes with `Exception 'Koha::Exception' thrown 'test_background_job is not a valid job_type'` meaning the plugins didn't get initialized? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #26 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #25)
So it now explodes with
`Exception 'Koha::Exception' thrown 'test_background_job is not a valid job_type'`
meaning the plugins didn't get initialized?
Yeah, that was the same error I ended up dealing with. I didn't have time to puzzle it out. (In reply to David Cook from comment #21)
But I've wasted way too much time on this already... I don't think I can work on this anymore.
Apologies if this sounded harsh. I was just feeling really frustrated after struggling for a long time with the unit test. I really can't rationalize spending more time on this one, when I've got a lot of other things that I know I can make progress instead of spinning my wheels :/. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #27 from David Cook <dcook@prosentient.com.au> --- Still think the change overall is interesting though! I do think that some of the issues with the unit tests does suggest that this current strategy might not be enough, and that maybe we should try to plan/design this more carefully... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 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=36736 --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm tempted to move the tests to a new bug.. whilst it would be lovely to cover it I'm keen to get the fix into core sooner rather than later. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #29 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Martin Renvoize from comment #28)
I'm tempted to move the tests to a new bug.. whilst it would be lovely to cover it I'm keen to get the fix into core sooner rather than later.
+1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169410|0 |1 is obsolete| | Attachment #169508|0 |1 is obsolete| | --- Comment #30 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 169555 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169555&action=edit Bug 36736: Load plugins at the start of background job processing This patch adds a call to get_enabled_plugins before processing background jobs to ensure that all plugin hooks are loaded and cached To test: 1 - Install a plugin that adds new objects e.g. the Contracts plugin https://github.com/bywatersolutions/fs-koha-plugin-contracts or the Koha Advent plugin: https://gitlab.com/koha-community/koha-advent/koha-plugin-fancyplugin 2 - Restart all 3 - Tail all your logs 4 - Stage and import a file containing items 5 - Note in the logs DBI Exception: DBD::mysql::st execute failed: Lock wait timeout exceeded; try restarting transaction 6 - Apply this patch 7 - Restart all 8 - Stage and import again 9 - Success! Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Tomás Cohen Arazi <tomascohen@gmail.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=36736 --- Comment #31 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #28)
I'm tempted to move the tests to a new bug.. whilst it would be lovely to cover it I'm keen to get the fix into core sooner rather than later.
+1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #32 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Is that really major or not more an enh? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm, the title of the bug and the content of the patch don't seem to match up. Maybe we should update the bug description? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #34 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! 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=36736 --- Comment #35 from David Cook <dcook@prosentient.com.au> --- I'm curious how this would interact with bug 36721, as I preload Koha::Schema there into the Starman master process. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #36 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to David Cook from comment #35)
I'm curious how this would interact with bug 36721, as I preload Koha::Schema there into the Starman master process.
My instinct is that your patch won't interact with this one, as the only module you load from the Koha namespace is Koha::Schema which itself doesn't anything else in the Koha or C4 namespaces. I suppose that begs the question of "should we include Koha/C4 packages in Preload.pm" but that's more of a question to be answered on that bug :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #37 from David Cook <dcook@prosentient.com.au> --- (In reply to Kyle M Hall from comment #36)
(In reply to David Cook from comment #35)
I'm curious how this would interact with bug 36721, as I preload Koha::Schema there into the Starman master process.
My instinct is that your patch won't interact with this one, as the only module you load from the Koha namespace is Koha::Schema which itself doesn't anything else in the Koha or C4 namespaces. I suppose that begs the question of "should we include Koha/C4 packages in Preload.pm" but that's more of a question to be answered on that bug :)
Yeah, I don't think it will conflict either. I figure your plugin just adds on top of the Koha::Schema::ResultSet classes, so it should be fine... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #38 from Jonathan Druart <jonathan.druart@gmail.com> --- looks like we are still waiting for tests here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38507 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38507 [Bug 38507] Add tests for changes from bug 36736 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #38)
looks like we are still waiting for tests here.
Moved to separate bug 38507. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz Version(s)|24.11.00 |24.11.00,24.05.11 released in| | Status|Pushed to oldstable |Needs documenting -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 --- Comment #41 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED CC| |david@davidnind.com --- Comment #42 from David Nind <david@davidnind.com> --- Developer focused, not covered in the manual - no update required. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36736 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38384 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38384 [Bug 38384] General fix for plugins breaking database transactions -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org