Greetings, I was reading misc/cronjobs/advance_notices.pl and was trying to make sense of where $upcoming_digest is set. I think there must be some side-effect obscuring the code. I was wondering if a second pair of eyes might find it. I was trying to determine how $digest->{email} gets set for the from_address call to C4::Letters::EnqueueLetter. GPML, Mark Tompsett
Holà Koha! After so many rebasing nightmares only caused by updatedatabase.pl, I was wondering how you are doing your insertions in that file to make things easier. Git is a bit dumb by default regarding that file, so I was wondering if you knew of any way to write the insertion to always show at the right place in a rebase. Of course, maybe it's just hopeless, in which case may I ask why we are doing it this way? Locally, I moved from that file and instead create a different file for each update (update_XXXX_long_description), and have my script run them all in sorted order. I could see that used in the community, with a subdirectory for each version. I can imagine it would have a very small impact on performance, but it'd be sooo much easier to rebase. Thanks, Blou Rebase, rebase, rebase....
On 19 November 2014 10:48, Philippe Blouin <philippe.blouin@inlibro.com> wrote:
Holà Koha! After so many rebasing nightmares only caused by updatedatabase.pl, I was wondering how you are doing your insertions in that file to make things easier. Git is a bit dumb by default regarding that file, so I was wondering if you knew of any way to write the insertion to always show at the right place in a rebase.
Of course, maybe it's just hopeless, in which case may I ask why we are doing it this way?
Locally, I moved from that file and instead create a different file for each update (update_XXXX_long_description), and have my script run them all in sorted order. I could see that used in the community, with a subdirectory for each version. I can imagine it would have a very small impact on performance, but it'd be sooo much easier to rebase.
There's a patch to do just that, http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13068 you could test and sign off
Chris
I started looking at DBIx::Class::Schema::deploy() and DBIx::Class::Schema::Versioned::upgrade() the other day, as I would love us to move away from kohastructure.sql and updatedatabase.pl. Galen’s existing work is available on Bugzilla <http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11390> http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11390 , as are some of my comments about my investigations into using these methods. I’m having a few minor issues at the moment, but hopefully I’ll resolve them soon. DBIx::Class::Schema has a method called create_ddl_dir(), which lets you dump the DBIC schema out as a SQL file. It also allows you to create SQL “diffs”, which operate as version updates. So the method will save a full dump, plus the diff. That was you can run incremental updates in sorted order, or just re-deploy the entire database at any specific version. Of course, it still requires us to write a script to handle when these things occur, but that should be easy. That said, it still runs into the same problems as bug 13068. Namely, it’s just SQL updates. If we need anything more complex using Perl, I think we’ll need to write specific update scripts, or a different kind of “updatedatabase.pl” which uses DBIx::Class::Schema::Versioned for the heavy lifting and itself for more of the delicate/sophisticated updates. Anyway, that’s my 2 cents : ) David Cook Systems Librarian Prosentient Systems 72/330 Wattle St, Ultimo, NSW 2007 From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel-bounces@lists.koha-community.org] On Behalf Of Chris Cormack Sent: Wednesday, 19 November 2014 8:52 AM To: koha-devel@lists.koha-community.org Subject: Re: [Koha-devel] Any secret for updatedatabase.pl? On 19 November 2014 10:48, Philippe Blouin <philippe.blouin@inlibro.com <mailto:philippe.blouin@inlibro.com> > wrote: Holà Koha! After so many rebasing nightmares only caused by updatedatabase.pl <http://updatedatabase.pl> , I was wondering how you are doing your insertions in that file to make things easier. Git is a bit dumb by default regarding that file, so I was wondering if you knew of any way to write the insertion to always show at the right place in a rebase. Of course, maybe it's just hopeless, in which case may I ask why we are doing it this way? Locally, I moved from that file and instead create a different file for each update (update_XXXX_long_description), and have my script run them all in sorted order. I could see that used in the community, with a subdirectory for each version. I can imagine it would have a very small impact on performance, but it'd be sooo much easier to rebase. There's a patch to do just that, http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13068 you could test and sign off Chris
If we need anything more complex using Perl, I think we’ll need to write specific update scripts, or a
different kind of “updatedatabase.pl” which uses DBIx::Class::Schema::Versioned for the heavy
lifting and itself for more of the delicate/sophisticated updates.
Bug 13068 allows you to put a perl script in the atomicupdate directory. So why not put our new DBIC-style database revisions there?
Hello Mark, I believe that you are the good candidate to sign off on bug 13240 ( advanced_notices.pl contains code obfuscation) :) Cheers, Jonathan 2014-11-18 22:27 GMT+01:00 Mark Tompsett <mtompset@hotmail.com>:
Greetings,
I was reading misc/cronjobs/advance_notices.pl and was trying to make sense of where $upcoming_digest is set. I think there must be some side-effect obscuring the code. I was wondering if a second pair of eyes might find it. I was trying to determine how $digest->{email} gets set for the from_address call to C4::Letters::EnqueueLetter.
GPML, Mark Tompsett
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
Mark,
You were spot on when you said "there must be some side-effect obscuring the code" $upcoming_digest is set on line 281: http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=misc/cronjobs/adva... Here's the code snippit 281 my $digest = $upcoming_digest->{$upcoming->{'borrowernumber'}} ||= {}; This is saying If $upcoming_digest->{$upcoming->{'borrowernumber'}} is false, set it to an empy hashref. Then set the $digest to the value of $upcoming_digest->{$upcoming->{'borrowernumber'}}. It just so happens that advance_notices.pl was the first piece of Koha code that I read through in detail ,and I remember griping about it in #koha: http://irc.koha-community.org/koha/2014-01-22#i_1455327 Your reply was 15:39 mtompset > Oh my! That is perl abuse. :P ;-) ;-) Unfortunately, I didn't file a bug report as suggested by tcohen. --Barton On Tue, Nov 18, 2014 at 4:27 PM, Mark Tompsett <mtompset@hotmail.com> wrote:
Greetings,
I was reading misc/cronjobs/advance_notices.pl and was trying to make sense of where $upcoming_digest is set. I think there must be some side-effect obscuring the code. I was wondering if a second pair of eyes might find it. I was trying to determine how $digest->{email} gets set for the from_address call to C4::Letters::EnqueueLetter.
GPML, Mark Tompsett
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
participants (7)
-
Barton Chittenden -
Chris Cormack -
David Cook -
Jonathan Druart -
Marcel de Rooy -
Mark Tompsett -
Philippe Blouin