[Bug 36419] New: Ambiguous return value in GetPlugins
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36419 Bug ID: 36419 Summary: Ambiguous return value in GetPlugins Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org Bug 35536 introduced some better error handling, but made the return value more erratic. We you previously had code like this: ```perl my @plugins = Koha::Plugins->new()->GetPlugins( { method => 'ill_backend', metadata => { name => $backend_id }, all => 1, errors => 1 } ); foreach my $plugin (@plugins) { ... } ``` And now the new return value for GetPlugins is: ```perl return $errors ? ( \@plugins, \@failing ) : @plugins; ``` This means the caller will need to check the return value, and if the arity is two (?), check that the second element has some specific structure so we determine... it is not actually a plugin! I propose we return a hashref with the plugins and (possibly) the errors. -- 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=36419 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com, | |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Depends on| |35536 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35536 [Bug 35536] Improve removal of Koha plugins in unit tests -- 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=36419 Tomás Cohen Arazi <tomascohen@gmail.com> 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=36419 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36419 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I have now noticed that the `errors => 1` from the original code was the one preventing it to work. So I'll lower the importance to enhancement, but want to propose a signature change here. For discussion. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36419 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 163850 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163850&action=edit [POC] Bug 36419: Use wantarray to control return value 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=36419 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |enhancement Status|ASSIGNED |In Discussion -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36419 --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 163895 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163895&action=edit Bug 36419: Use wantarray to control return value Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <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=36419 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163850|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36419 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Looks good to me! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36419 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Failed QA --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Idea is fine with me, current implementation would fail. Should we only return this good, bad (and ugly) if the errors param was passed? Otherwise we will need to examine all calls of GetPlugins without errors => 1 too. Plugins.t fails on a call now with errors, expecting the old returns. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36419 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |In Discussion --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #5)
Idea is fine with me, current implementation would fail. Should we only return this good, bad (and ugly) if the errors param was passed? Otherwise we will need to examine all calls of GetPlugins without errors => 1 too.
Plugins.t fails on a call now with errors, expecting the old returns.
Correct! I believe this patch is just a POC and not expected to fully function. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36419 Bug 36419 depends on bug 35536, which changed state. Bug 35536 Summary: Improve removal of Koha plugins in unit tests https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35536 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org