Hi,
We used to have problems with transactions, if they were used from the
a subroutine/method which was called from tests (which use
transactions as well), it did not work.
Two transactions could not be started together.
I recall nested transactions were problematic.
Now that we are using DBIx::Class transactions it works correctly and
we will be able to use it in the codebase everywhere we need it.
I try to use it as soon as it's useful (see
Koha::Acquisition::Currency->store and bug 16907
Koha::Patrons->delete).
Yes! The point of my email, was to highlight the possibility we now have, and gather opinions on how to better do it. I wanted to raise this on a dev meeting. We have a tradition of letting patches speak, but some healthy discussion about this could let us reach some consensus, and guide new devs into good coding practices.
Regarding the try-catch module, the following is fine by me:
use Koha::Exceptions;
use Try::Tiny;
sub boom {
Koha::Exceptions::Exception->throw;
}
try {
boom();
} catch {
if ( ref($_) eq 'Koha::Exceptions::Exception' ) {
# Do that
} elsif ( ref($_) eq 'Koha::Exceptions::AnotherException' ) {
# do something else
}
}
Yes, I like the Try::Tiny lib and would propose to just start using it. Your code is simple to read, and a clear improvement! Koha::Virtualshelf already raises exceptions and
shelves.pl catches them on another way, in master!
I mentioned TryCatch only because its syntax is sexier IMHO, but the main point was, again, to highlight the possible use of some convenient try/catch lib on the Koha:: namespace, which in conjuntcion with transactions would make our code quite reliable and readable.
You should also have a look at Bug 13995 - Proper Exception handling
I will, I recall the bug got stuck because Olli introduced one file per exception. It is handy for editor's autocomplete features, but it looked awkward. That's why I propose splitting exceptions files on a per-package basis.
Cheers