[Bug 36473] New: updatetotalissues.pl shoudl not die on a bad record
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Bug ID: 36473 Summary: updatetotalissues.pl shoudl not die on a bad record 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: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org See bug 36013 comment 3 -- 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=36473 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=36473 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 164192 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164192&action=edit Bug 36473: Handle Invalid Metadata Exceptions in totalissues.pl This patch wraps the call for record in an eval, and catches any invalid metadata exceptions, letting the warning show, but allowing the script to continue To test: 1 - In default KTD record 369 has problems, otherwise you need to break a record 2 - Run : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v 3 - It dies at record 369 (or the one you broke) 4 - Apply patch 5 - Run again 6 - It succeeds, but skips the bad record -- 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=36473 Nick Clemens <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 the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36013 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|updatetotalissues.pl shoudl |updatetotalissues.pl should |not die on a bad record |not die on a bad record -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36474 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36474 [Bug 36474] updatetotalissues.pl should not modify the record when the total issues has not changed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 David Nind <david@davidnind.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=36473 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164192|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 164247 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164247&action=edit Bug 36473: Handle Invalid Metadata Exceptions in totalissues.pl This patch wraps the call for record in an eval, and catches any invalid metadata exceptions, letting the warning show, but allowing the script to continue To test: 1 - In default KTD record 369 has problems, otherwise you need to break a record 2 - Run : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v 3 - It dies at record 369 (or the one you broke) 4 - Apply patch 5 - Run again 6 - It succeeds, but skips the bad record Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Text to go in the| |This fixes the release notes| |misc/cronjobs/update_totali | |ssues.pl script so that it | |skips records with invalid | |data, instead of just | |stopping. (This also means | |that the 'Most-circulated | |items' report now shows the | |correct data - if the | |script stopped because of | |invalid records, the report | |may have not picked up | |circulated items.) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164247|0 |1 is obsolete| | --- Comment #3 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 164269 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164269&action=edit Bug 36473: Handle Invalid Metadata Exceptions in totalissues.pl This patch wraps the call for record in an eval, and catches any invalid metadata exceptions, letting the warning show, but allowing the script to continue To test: 1 - In default KTD record 369 has problems, otherwise you need to break a record 2 - Run : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v 3 - It dies at record 369 (or the one you broke) 4 - Apply patch 5 - Run again 6 - It succeeds, but skips the bad record -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 --- Comment #4 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 164270 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164270&action=edit Bug 36473: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164269|0 |1 is obsolete| | --- Comment #5 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 164271 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164271&action=edit Bug 36473: Handle Invalid Metadata Exceptions in totalissues.pl This patch wraps the call for record in an eval, and catches any invalid metadata exceptions, letting the warning show, but allowing the script to continue To test: 1 - In default KTD record 369 has problems, otherwise you need to break a record 2 - Run : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v 3 - It dies at record 369 (or the one you broke) 4 - Apply patch 5 - Run again 6 - It succeeds, but skips the bad record -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164270|0 |1 is obsolete| | --- Comment #6 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 164289 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164289&action=edit Bug 36473: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164271|0 |1 is obsolete| | --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 164290 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164290&action=edit Bug 36473: Handle Invalid Metadata Exceptions in totalissues.pl This patch wraps the call for record in an eval, and catches any invalid metadata exceptions, letting the warning show, but allowing the script to continue To test: 1 - In default KTD record 369 has problems, otherwise you need to break a record 2 - Run : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v 3 - It dies at record 369 (or the one you broke) 4 - Apply patch 5 - Run again 6 - It succeeds, but skips the bad record Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) { UpdateTotalIssues( $item_object->biblionumber, 1, undef, { skip_holds_queue => 1 } ); } IIUC AddIssue will no longer crash on an invalid metadata record, right? Is that really what we want? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164289|0 |1 is obsolete| | --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 164458 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164458&action=edit Bug 36473: Unit tests Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] Tidied. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164290|0 |1 is obsolete| | --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 164459 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164459&action=edit Bug 36473: Handle Invalid Metadata Exceptions in totalissues.pl This patch wraps the call for record in an eval, and catches any invalid metadata exceptions, letting the warning show, but allowing the script to continue To test: 1 - In default KTD record 369 has problems, otherwise you need to break a record 2 - Run : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v 3 - It dies at record 369 (or the one you broke) 4 - Apply patch 5 - Run again 6 - It succeeds, but skips the bad record Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Changing status for feedback -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #8)
if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) { UpdateTotalIssues( $item_object->biblionumber, 1, undef, { skip_holds_queue => 1 } ); }
IIUC AddIssue will no longer crash on an invalid metadata record, right? Is that really what we want?
This was an unintended consequence, however, I do think this is what we want - there will be a warning in the logs - but why should we prevent checking out an item with a bad record? We should be handling that on creation or editing of a record. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164458|0 |1 is obsolete| | --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 164805 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164805&action=edit Bug 36473: Unit tests Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] Tidied. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164459|0 |1 is obsolete| | --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 164806 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164806&action=edit Bug 36473: Handle Invalid Metadata Exceptions in totalissues.pl This patch wraps the call for record in an eval, and catches any invalid metadata exceptions, letting the warning show, but allowing the script to continue To test: 1 - In default KTD record 369 has problems, otherwise you need to break a record 2 - Run : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v 3 - It dies at record 369 (or the one you broke) 4 - Apply patch 5 - Run again 6 - It succeeds, but skips the bad record Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <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=36473 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |24.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! 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=36473 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- It looks like this might make the tests fail (needs further investigation): t_db_dependent_Biblio_t.UpdateTotalIssues on Invalid record (2144) Error Details not ok 21 - UpdateTotalIssues on Invalid record -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I have a hunch that this broke tests: prove t/db_dependent/Biblio.t # Failed test 'Expected warning found' # at t/db_dependent/Biblio.t line 1039. # found warning: UpdateTotalIssues could not get bibliographic record at /kohadevbox/koha/C4/Biblio.pm line 3065. # expected to find warning: UpdateTotalIssues could not get biblio record # Looks like you failed 1 test of 2. # Failed test 'UpdateTotalIssues on Invalid record' # at t/db_dependent/Biblio.t line 1043. # Looks like you failed 1 test of 21. t/db_dependent/Biblio.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/21 subtests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #18 from Jonathan Druart <jonathan.druart@gmail.com> --- I don't understand this fix. Why don't you eval/try-catch in the script instead of inside UpdateTotalIssues? Also the warn should display the error, otherwise you are potentially hidden useful warnings. Please fix ASAP, Jenkins is failing! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 --- Comment #19 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 165039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165039&action=edit Bug 36473: [ALTERNATIVE PATCH] Prevent UpdateTotalIssues failure to crash update_totalissues.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 --- Comment #20 from Jonathan Druart <jonathan.druart@gmail.com> --- This is how I would have fixed it initially. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165039|0 |1 is obsolete| | --- Comment #21 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 165077 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165077&action=edit Bug 36473: (follow-up) Correct biblio to bibliographic -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 --- Comment #22 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 165078 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165078&action=edit Bug 36473: (follow-up) Add biblionumber to warning -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 --- Comment #23 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 165079 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165079&action=edit Bug 36473: (follow-up) Additionally warn the stringified exception -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | --- Comment #24 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- All 3 follow-ups have been pushed to main: https://git.koha-community.org/Koha-community/Koha/commits/branch/main/searc... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.05.00 |24.05.00,23.11.06 released in| | Status|Pushed to main |Pushed to stable CC| |fridolin.somers@biblibre.co | |m --- Comment #25 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Version(s)|24.05.00,23.11.06 |24.05.00,23.11.06,23.05.12 released in| | Status|Pushed to stable |Pushed to oldstable --- Comment #26 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.12 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36473 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED CC| |wainuiwitikapark@catalyst.n | |et.nz Resolution|--- |FIXED --- Comment #27 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 unless requested -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org