[Bug 34881] New: Database update for bug 28854 isn't fully idempotent
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 Bug ID: 34881 Summary: Database update for bug 28854 isn't fully idempotent Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Installation and upgrade (command-line installer) Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org Depends on: 28854 The AV added as part of bug 28854 can be duplicated if the update is run more than once. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28854 [Bug 28854] Add ability to create bundles of items for circulation -- 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=34881 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 156047 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156047&action=edit Bug 34881: Improve idempotency of DBRev -- 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=34881 Martin Renvoize <martin.renvoize@ptfs-europe.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=34881 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.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=34881 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=34881 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- I think this part + $dbh->do(qq{ + INSERT IGNORE INTO systempreferences( `variable`, `value`, `options`, `explanation`, `type` ) + VALUES + ( 'BundleLostValue', $lost_val, '', 'Sets the LOST AV value that represents "Missing from bundle" as a lost value', 'Free' ), + ( 'BundleNotLoanValue', $nfl_val, '', 'Sets the NOT_LOAN AV value that represents "Added to bundle" as a not for loan value', 'Free') + }); + + say $out "Added new system preference 'BundleLostValue'"; + say $out "Added new system preference 'BundleNotLoanValue'"; should not be in the if you added. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- It can't not be.. it depends on a variable added within the IF -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #4 from David Nind <david@davidnind.com> --- Just checking on what is the best way to test this. I tried these steps, but not sure if this is the best way: 1. Check the authorized values list for LOST and note that there is this value: 5 Missing from bundle 2. Run the database update (found this on another idempotent bug): perl -e 'use C4::Installer; warn Data::Dumper::Dumper(C4::Installer::run_db_rev("installer/data/mysql/db_revs/220600015.pl"));' 3. Note that for the LOST authorized value lists, the value is duplicated: 6 Missing from bundle 4. Reset everything: reset_all 5. Run the database update again (see step 2). This time I got this error when running the update (at least the value isn't duplicated!): perl -e 'use C4::Installer; warn Data::Dumper::Dumper(C4::Installer::run_db_rev("installer/data/mysql/db_revs/220600015.pl"));' $VAR1 = { 'error' => bless( { 'msg' => '{UNKNOWN}: DBI Exception: DBD::mysql::db selectrow_array failed: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near \'\' at line 1 at /kohadevbox/koha/C4/Installer.pm line 741 ' }, 'DBIx::Class::Exception' ), 'time' => '18:19:10', 'exec_output' => 'Added new table \'item_bundles\' ', 'bug_number' => '28854', 'output' => [ 'Upgrade to 22.06.00.015 [18:19:10]: Bug 28854 - Item bundles support', ' Added new table \'item_bundles\'' ], 'description' => 'Item bundles support', 'version' => '22.06.00.015', 'filepath' => 'installer/data/mysql/db_revs/220600015.pl' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov --- Comment #5 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- The subquery in line 24 of the patch is missing its close parenthesis at the end - that's probably what's causing the error. I agree that it feels off to put the system preference revisions (and the not-for-loan AV revision for that matter) all inside of the IF statement that depends only on the Lost AV. Can we safely assume that either the whole dbrev has been run, or none of it has been run? If so, why not put everything inside the if block that's already there: 10 if( !TableExists( 'item_bundles' ) ) { If not, then shouldn't we check each thing separately? Uploading an alternative patch for how this could work... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 --- Comment #6 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 156823 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156823&action=edit Bug 34881: [alternative patch] Improve idempotency of DBRev -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 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=34881 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #156823|0 |1 is obsolete| | --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 156878 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156878&action=edit Bug 34881: [alternative patch] Improve idempotency of DBRev 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=34881 --- Comment #8 from David Nind <david@davidnind.com> --- Thanks for the alternative patch Emily! I've signed off as I don't get an error when running the database update. If it shouldn't be signed off yet, please change the status back to needs sign-off. Running the database update - first time ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Following the test plan in #c4 I no longer get an error message when I run the database update for the first time: perl -e 'use C4::Installer; warn Data::Dumper::Dumper(C4::Installer::run_db_rev("installer/data/mysql/db_revs/220600015.pl"));' $VAR1 = { 'output' => [ 'Upgrade to 22.06.00.015 [20:11:52]: Bug 28854 - Item bundles support', ' Added new table \'item_bundles\'', ' Added new system preference \'BundleLostValue\'', ' Added new system preference \'BundleNotLoanValue\'' ], 'filepath' => 'installer/data/mysql/db_revs/220600015.pl', 'version' => '22.06.00.015', 'exec_output' => 'Added new table \'item_bundles\' Added new system preference \'BundleLostValue\' Added new system preference \'BundleNotLoanValue\' ', 'description' => 'Item bundles support', 'bug_number' => '28854', 'error' => undef, 'time' => '20:11:52' Running the database update - second time ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ perl -e 'use C4::Installer; warn Data::Dumper::Dumper(C4::Installer::run_db_rev("installer/data/mysql/db_revs/220600015.pl"));' $VAR1 = { 'output' => [ 'Upgrade to 22.06.00.015 [20:11:52]: Bug 28854 - Item bundles support', ' Added new table \'item_bundles\'', ' Added new system preference \'BundleLostValue\'', ' Added new system preference \'BundleNotLoanValue\'' ], 'filepath' => 'installer/data/mysql/db_revs/220600015.pl', 'version' => '22.06.00.015', 'exec_output' => 'Added new table \'item_bundles\' Added new system preference \'BundleLostValue\' Added new system preference \'BundleNotLoanValue\' ', 'description' => 'Item bundles support', 'bug_number' => '28854', 'error' => undef, 'time' => '20:11:52' }; Running the database update - third and subsequent times ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ perl -e 'use C4::Installer; warn Data::Dumper::Dumper(C4::Installer::run_db_rev("installer/data/mysql/db_revs/220600015.pl"));' $VAR1 = { 'bug_number' => '28854', 'exec_output' => 'Added new table \'item_bundles\' Added new system preference \'BundleLostValue\' Added new system preference \'BundleNotLoanValue\' ', 'filepath' => 'installer/data/mysql/db_revs/220600015.pl', 'error' => undef, 'version' => '22.06.00.015', 'time' => '20:15:11', 'output' => [ 'Upgrade to 22.06.00.015 [20:15:11]: Bug 28854 - Item bundles support', ' Added new table \'item_bundles\'', ' Added new system preference \'BundleLostValue\'', ' Added new system preference \'BundleNotLoanValue\'' ], 'description' => 'Item bundles support' }; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 Nick Clemens <nick@bywatersolutions.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=34881 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #156047|0 |1 is obsolete| | Attachment #156878|0 |1 is obsolete| | --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 156978 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156978&action=edit Bug 34881: Improve idempotency of DBRev Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 156979 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156979&action=edit Bug 34881: (QA follow-up) Remove stray paren Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|martin.renvoize@ptfs-europe |emily.lamancusa@montgomeryc |.com |ountymd.gov CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.11.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=34881 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00 |23.11.00,23.05.05 released in| | CC| |fridolin.somers@biblibre.co | |m Status|Pushed to master |Pushed to stable --- Comment #12 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.05.x for 23.05.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00,23.05.05 |23.11.00,23.05.05,22.11.12 released in| | Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 --- Comment #13 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to oldstable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34881 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to oldstable |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org