Bugzilla – Attachment 123356 Details for
Bug 25078
Update DB process - wrap each DBRev inside a transaction and better error handling
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25078: Keep atomic updates in "atomicupdate" dir
Bug-25078-Keep-atomic-updates-in-atomicupdate-dir.patch (text/plain), 16.97 KB, created by
Jonathan Druart
on 2021-08-02 10:15:59 UTC
(
hide
)
Description:
Bug 25078: Keep atomic updates in "atomicupdate" dir
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2021-08-02 10:15:59 UTC
Size:
16.97 KB
patch
obsolete
>From cfb2b92addbe265248eb9bc508eaf42ad9f2c262 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Mon, 2 Aug 2021 11:06:57 +0200 >Subject: [PATCH] Bug 25078: Keep atomic updates in "atomicupdate" dir > >The atomicupdate directory will contain all the atomic update files (old >and new version). >That will ease job for the RM and RMaints, no file (except skeletons) >must be in this directory before push. > >This patch also fixes an inconsistency we had: the atomic update was run >before the other db revs on the UI but after when the CLI script was >used. > >Now we make sure that the CLI does not deal with the atomic update files >when called from the installer (UI) >--- > C4/Installer.pm | 186 +++++++++++++----- > .../data/mysql/atomicupdate/bug_25078.perl | 3 +- > installer/data/mysql/updatedatabase.pl | 29 ++- > installer/install.pl | 11 +- > .../prog/en/modules/installer/step3.tt | 55 +++++- > 5 files changed, 206 insertions(+), 78 deletions(-) > >diff --git a/C4/Installer.pm b/C4/Installer.pm >index ad2fe3e33c9..dff541732ec 100644 >--- a/C4/Installer.pm >+++ b/C4/Installer.pm >@@ -23,16 +23,18 @@ use Try::Tiny; > use Encode qw( encode decode is_utf8 ); > use DBIx::RunSQL; > use YAML::XS; >+use File::Slurp qw( read_file ); >+use DBI; >+ > use C4::Context; > use Koha::Schema; >-use DBI; > use Koha; > > use vars qw(@ISA @EXPORT); > BEGIN { > require Exporter; > @ISA = qw( Exporter ); >- push @EXPORT, qw( primary_key_exists foreign_key_exists index_exists column_exists TableExists TransformToNum CheckVersion NewVersion SetVersion sanitize_zero_date update get_db_entries ); >+ push @EXPORT, qw( primary_key_exists foreign_key_exists index_exists column_exists TableExists TransformToNum CheckVersion NewVersion SetVersion sanitize_zero_date update get_db_entries get_atomic_updates run_atomic_updates ); > }; > > =head1 NAME >@@ -711,44 +713,54 @@ sub get_db_entries { > return \@need_update; > } > >+sub run_db_rev { >+ my ($file) = @_; >+ >+ my $db_rev = do $file; >+ >+ my $error; >+ my $out = ''; >+ open my $outfh, '>', \$out; >+ try { >+ my $schema = Koha::Database->new->schema; >+ $schema->txn_do( >+ sub { >+ $db_rev->{up}->( { dbh => $schema->storage->dbh, out => $outfh } ); >+ } >+ ); >+ } >+ catch { >+ $error = $_; >+ }; >+ >+ close $outfh; >+ $out = decode( 'UTF-8', $out ); >+ >+ my $db_entry = { >+ filepath => $file, >+ bug_number => $db_rev->{bug_number}, >+ description => $db_rev->{description}, >+ exec_output => $out, >+ version => scalar version_from_file($file), >+ time => POSIX::strftime( "%H:%M:%S", localtime ), >+ error => $error >+ }; >+ $db_entry->{output} = generate_output_db_entry($db_entry, $out); >+ return $db_entry; >+} >+ > sub update { > my ( $files, $params ) = @_; > > my $force = $params->{force} || 0; > >- my $schema = Koha::Database->new->schema; > my ( @done, @errors ); > for my $file ( @$files ) { > >- my $db_rev = do $file; >- >- my $error; >- >- my $out = ''; >- open my $outfh, '>', \$out; >- try { >- $schema->txn_do( >- sub { >- $db_rev->{up}->({ dbh => $schema->storage->dbh, out => $outfh }); >- } >- ); >- } catch { >- $error = $_; >- }; >- >- close $outfh; >- $out = decode('UTF-8', $out); >- >- my $db_entry = { >- bug_number => $db_rev->{bug_number}, >- description => $db_rev->{description}, >- version => version_from_file($file), >- time => POSIX::strftime( "%H:%M:%S", localtime ), >- }; >- $db_entry->{output} = output_version( { %$db_entry, done => !$error, report => $out } ); >+ my $db_entry = run_db_rev($file); > >- if ( $error ) { >- push @errors, { %$db_entry, error => $error }; >+ if ( $db_entry->{error} ) { >+ push @errors, $db_entry; > $force ? next : last ; > # We stop the update if an error occurred! > } >@@ -759,30 +771,39 @@ sub update { > return { success => \@done, error => \@errors }; > } > >-sub output_version { >+sub generate_output_db_entry { > my ( $db_entry ) = @_; > > my $description = $db_entry->{description}; >- my $report = $db_entry->{report}; >- my $DBversion = $db_entry->{version}; >- my $bug_number = $db_entry->{bug_number}; >- my $time = $db_entry->{time}; >- my $done = defined $db_entry->{done} >- ? $db_entry->{done} >- ? " done" >- : " failed" >- : ""; # For old versions, we don't know if we succeed or failed >+ my $output = $db_entry->{output}; >+ my $DBversion = $db_entry->{version}; >+ my $bug_number = $db_entry->{bug_number}; >+ my $time = $db_entry->{time}; >+ my $exec_output = $db_entry->{exec_output}; >+ my $done = defined $db_entry->{done} >+ ? $db_entry->{done} >+ ? " done" >+ : " failed" >+ : ""; # For old versions, we don't know if we succeed or failed > > my @output; > >- if ($bug_number) { >- push @output, sprintf('Upgrade to %s %s [%s]: Bug %5s - %s', $DBversion, $done, $time, $bug_number, $description); >- } else { >- push @output, sprintf('Upgrade to %s %s [%s]: %s', $DBversion, $done, $time, $description); >+ if ( $DBversion ) { >+ if ($bug_number) { >+ push @output, sprintf('Upgrade to %s %s [%s]: Bug %5s - %s', $DBversion, $done, $time, $bug_number, $description); >+ } else { >+ push @output, sprintf('Upgrade to %s %s [%s]: %s', $DBversion, $done, $time, $description); >+ } >+ } else { # Atomic update >+ if ($bug_number) { >+ push @output, sprintf('DEV atomic update %s %s [%s]: Bug %5s - %s', $db_entry->{filepath}, $done, $time, $bug_number, $description); >+ } else { # Old atomic update syntax >+ push @output, sprintf('DEV atomic update %s %s [%s]', $db_entry->{filepath}, $done, $time); >+ } > } > >- if ($report) { >- foreach my $line (split /\n/, $report) { >+ if ($exec_output) { >+ foreach my $line (split /\n/, $exec_output) { > push @output, sprintf "\t\t\t\t\t\t - %s", $line; > } > } >@@ -790,6 +811,75 @@ sub output_version { > return \@output; > } > >+sub get_atomic_updates { >+ my @atomic_upate_files; >+ # if there is anything in the atomicupdate, read and execute it. >+ my $update_dir = C4::Context->config('intranetdir') . '/installer/data/mysql/atomicupdate/'; >+ opendir( my $dirh, $update_dir ); >+ foreach my $file ( sort readdir $dirh ) { >+ next if $file !~ /\.(perl|pl)$/; #skip other files >+ next if $file eq 'skeleton.perl' || $file eq 'skeleton.pl'; # skip the skeleton files >+ >+ push @atomic_upate_files, $file; >+ } >+ return \@atomic_upate_files; >+} >+ >+sub run_atomic_updates { >+ my ( $files ) = @_; >+ >+ my $update_dir = C4::Context->config('intranetdir') . '/installer/data/mysql/atomicupdate/'; >+ my ( @done, @errors ); >+ for my $file ( @$files ) { >+ my $filepath = $update_dir . $file; >+ >+ my $atomic_update; >+ if ( $file =~ m{\.perl$} ) { >+ my $code = read_file( $filepath ); >+ my ( $out, $err ) = ('', ''); >+ { >+ open my $oldout, ">&STDOUT"; >+ close STDOUT; >+ open STDOUT,'>:encoding(utf8)', \$out; >+ my $DBversion = Koha::version; # We need $DBversion and $dbh for the eval >+ my $dbh = C4::Context->dbh; >+ eval $code; ## no critic (StringyEval) >+ $err = $@; >+ warn $err if $err; >+ close STDOUT; >+ open STDOUT, ">&", $oldout; >+ } >+ >+ $atomic_update = { >+ filepath => $filepath, >+ description => '', >+ version => undef, >+ time => POSIX::strftime( "%H:%M:%S", localtime ), >+ }; >+ >+ >+ $atomic_update->{output} = >+ $out >+ ? [ split "\n", $out ] >+ : generate_output_db_entry($atomic_update); # There wad an error, we didn't reach NewVersion) >+ >+ $atomic_update->{error} = $err if $err; >+ } elsif ( $file =~ m{\.pl$} ) { >+ $atomic_update = run_db_rev($filepath); >+ } else { >+ warn "Atomic update must be .perl or .pl ($file)"; >+ } >+ >+ if ( $atomic_update->{error} ) { >+ push @errors, $atomic_update; >+ } else { >+ push @done, $atomic_update; >+ } >+ } >+ >+ return { success => \@done, error => \@errors }; >+} >+ > =head2 DropAllForeignKeys($table) > > Drop all foreign keys of the table $table >@@ -874,7 +964,7 @@ sub NewVersion { > $description = $descriptions; > } > >- my $output = output_version( { >+ my $output = generate_output_db_entry( { > bug_number => $bug_number, > description => $description, > report => $report, >diff --git a/installer/data/mysql/atomicupdate/bug_25078.perl b/installer/data/mysql/atomicupdate/bug_25078.perl >index c76865211e8..8c89f5080b3 100644 >--- a/installer/data/mysql/atomicupdate/bug_25078.perl >+++ b/installer/data/mysql/atomicupdate/bug_25078.perl >@@ -7,7 +7,8 @@ if( CheckVersion( $DBversion ) ) { > # if( !column_exists( 'biblio', 'biblionumber' ) ) { > # warn "There is something wrong"; > # } >+ $dbh->do("SELECT * FROM pouet"); > > # Always end with this (adjust the bug info) >- NewVersion( $DBversion, 25078, ["Description", "Boom, boom", "Chick-a-boom"]); >+ NewVersion( $DBversion, 25078, ["DescriptxXXion", "Boom, boom", "Chick-a-boom"]); > } >diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl >index e9f2a538c20..4f89a31f604 100755 >--- a/installer/data/mysql/updatedatabase.pl >+++ b/installer/data/mysql/updatedatabase.pl >@@ -46,7 +46,6 @@ use MARC::Record; > use MARC::File::XML ( BinaryEncoding => 'utf8' ); > > use File::Path qw[remove_tree]; # perl core module >-use File::Slurp; > > # FIXME - The user might be installing a new database, so can't rely > # on /etc/koha.conf anyway. >@@ -24284,7 +24283,7 @@ if( CheckVersion( $DBversion ) ) { > } > > unless ( $ENV{HTTP_HOST} ) { # Is that correct? >- my $files = C4::Installer::get_db_entries; >+ my $files = get_db_entries; > my $report = update( $files, { force => $force } ); > > for my $s ( @{ $report->{success} } ) { >@@ -24294,24 +24293,18 @@ unless ( $ENV{HTTP_HOST} ) { # Is that correct? > say Encode::encode_utf8(join "\n", @{$e->{output}}); > say Encode::encode_utf8("ERROR - " . $e->{error}); > } >-} > >-# SEE bug 13068 >-# if there is anything in the atomicupdate, read and execute it. >-my $update_dir = C4::Context->config('intranetdir') . '/installer/data/mysql/atomicupdate/'; >-opendir( my $dirh, $update_dir ); >-foreach my $file ( sort readdir $dirh ) { >- next if $file !~ /\.(sql|perl)$/; #skip other files >- next if $file eq 'skeleton.perl'; # skip the skeleton file >- print "DEV atomic update: $file\n"; >- if ( $file =~ /\.sql$/ ) { >- my $installer = C4::Installer->new(); >- my $rv = $installer->load_sql( $update_dir . $file ) ? 0 : 1; >- } elsif ( $file =~ /\.perl$/ ) { >- my $code = read_file( $update_dir . $file ); >- eval $code; ## no critic (StringyEval) >- say "Atomic update generated errors: $@" if $@; >+ my $atomic_update_files = get_atomic_updates; >+ $report = run_atomic_updates($atomic_update_files); >+ for my $s ( @{ $report->{success} } ) { >+ say Encode::encode_utf8(join "\n", @{$s->{output}}); > } >+ for my $e ( @{ $report->{error} } ) { >+ say Encode::encode_utf8(join "\n", @{$e->{output}}); >+ say Encode::encode_utf8("ERROR - " . $e->{error}); >+ } >+ >+ > } > > exit; >diff --git a/installer/install.pl b/installer/install.pl >index a00e832e2d0..377730df7f2 100755 >--- a/installer/install.pl >+++ b/installer/install.pl >@@ -431,8 +431,17 @@ elsif ( $step && $step == 3 ) { > > my $db_entries = get_db_entries(); > my $report = update( $db_entries ); >+ my $atomic_update_files = get_atomic_updates; >+ my $atomic_update_report = run_atomic_updates( $atomic_update_files ); > >- $template->param( success => $report->{success}, error => $report->{error} ); >+ $template->param( >+ success => $report->{success}, >+ error => $report->{error}, >+ atomic_updates => { >+ success => $atomic_update_report->{success}, >+ error => $atomic_update_report->{error} >+ } >+ ); > > #warn "The following errors were returned while attempting to run the updatedatabase.pl script:\n"; #FIXME restore this > $template->param( $op => 1 ); >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt >index 2fca0c40dcf..b2a6604f4d0 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt >@@ -263,7 +263,7 @@ > [% END %] > > [%# Success for new versions %] >- [% IF success %] >+ [% IF success.size %] > <ul> > [% FOR s IN success %] > [% FOR o IN s.output %] >@@ -276,21 +276,55 @@ > </ul> > [% END %] > >- [%# Errors for old versions %] >- [% IF has_update_errors %] >- <p>Update errors :</p> >+ [% IF atomic_updates.success.size %] >+ <p>Atomic updates:</p> > <ul> >- [% FOREACH update_error IN update_errors %] >- <li class="update_error">[% update_error.line | html %]</li> >+ [% FOR s IN atomic_updates.success %] >+ [% FOR o IN s.output %] >+ <li>[% o | html %]</li> >+ [% IF s.output.size > 1 %] >+ [% IF loop.first %]<ul>[% ELSIF loop.last %]</ul>[% END %] >+ [% END %] >+ [% END %] > [% END %] > </ul> > [% END %] > >- [%# Errors for new versions %] >- [% IF error %] >- <p>Update error :</p> >+ [% IF has_update_errors OR error.size %] >+ <p>Update errors :</p> >+ [%# Errors for old versions %] >+ [% IF has_update_errors %] >+ <ul> >+ [% FOREACH update_error IN update_errors %] >+ <li class="update_error">[% update_error.line | html %]</li> >+ [% END %] >+ </ul> >+ [% END %] >+ >+ [%# Errors for new versions %] >+ [% IF error.size %] >+ <ul> >+ [% FOR e IN error %] >+ [% FOR o IN e.output %] >+ <li class="update_error"> >+ [% o | html %] >+ <br/> >+ ERROR: [% e.error | html %] >+ >+ [% IF e.output.size > 1 %] >+ [% IF loop.first %]<ul>[% ELSIF loop.last %]</ul>[% END %] >+ [% END %] >+ </li> >+ [% END %] >+ [% END %] >+ </ul> >+ [% END %] >+ [% END %] >+ >+ [% IF atomic_updates.error.size %] >+ <p>Atomic update error :</p> > <ul> >- [% FOR e IN error %] >+ [% FOR e IN atomic_updates.error %] > [% FOR o IN e.output %] > <li class="update_error"> > [% o | html %] >@@ -305,6 +339,7 @@ > [% END %] > </ul> > [% END %] >+ > [% UNLESS error OR has_update_errors %] > <p>Everything went okay. Update done.</p> > <p><a href="install.pl?step=3&op=finished" class="btn btn-primary">Continue to log in to Koha</a></p> >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 25078
:
102521
|
116032
|
116033
|
116034
|
116208
|
116209
|
117084
|
121909
|
121910
|
121911
|
121982
|
121983
|
121984
|
122208
|
122322
|
122323
|
122324
|
122325
|
122326
|
122329
|
122665
|
122666
|
122667
|
122668
|
122669
|
122670
|
122671
|
122679
|
122680
|
122875
|
123356
|
123357
|
123358
|
123359
|
123360
|
123361
|
123362
|
123363
|
123364
|
123365
|
123366
|
123367
|
123850
|
123851
|
123853
|
123937
|
124269