15:06:00 <tcohen> #startmeeting Development IRC meeting, 23 September 2014: DBIC 15:06:00 <huginn> Meeting started Tue Sep 23 15:06:00 2014 UTC. The chair is tcohen. Information about MeetBot at http://wiki.debian.org/MeetBot. 15:06:00 <huginn> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 15:06:00 <huginn> The meeting name has been set to 'development_irc_meeting__23_september_2014__dbic' 15:06:10 <gmcharlt> good morning 15:06:12 <tcohen> #topic Introductions 15:06:13 <wahanui> #info wahanui, a bot that has become sentient 15:06:14 <cait> good morning gmcharlt 15:06:25 <tcohen> please introduce yourselves using #info name 15:06:32 <ashimema> #info Martin Renvoize, PTFS Europe 15:06:34 <cait> #info Katrin Fischer, BSZ, Germany 15:06:39 <tcohen> #info Tomas Cohen Arazi, Universidad Nacional de Cordoba 15:06:56 <ColinC> #info Colin Campbell PTFS Europe Ltd 15:07:05 <ashimema> do we need to ping ribasushi at some point ;) 15:07:12 <gmcharlt> #info Galen Charlton, Equinox 15:07:12 <bgkriegel_> hi 15:07:36 <bgkriegel_> #info Bernardo Gonzalez Kriegel 15:07:54 <Joubu> #info Jonathan Druart, BibLibre 15:08:47 <tcohen> ok 15:08:54 <jcamins> #info Jared Camins-Esakov, C & P Bibliography Services 15:09:29 <tcohen> khall? 15:10:07 <tcohen> ok 15:10:26 <tcohen> This is an unusual meeting 15:10:31 <tcohen> to talk about a specific subject 15:10:56 <tcohen> #topic RM introduction 15:11:27 <oleonard> #info Owen Leonard, Athens County Public Libraries 15:11:34 <tcohen> The main subject is our adoption of DBIC, and how we do it 15:11:46 <tcohen> #link http://wiki.koha-community.org/wiki/DBIC_discussion_overview 15:12:27 <tcohen> the main discussion is related to how much stuff we expect DBIC to provide, and how to do it 15:12:46 <tcohen> there are mainly two discussed POV 15:13:25 <tcohen> on the one hand (a) benefit from DBIC magic, and put (some degree) of business logic into it 15:14:22 <tcohen> and on the other (b) relying on the ORM only for abstracting the DB and using it to implement our own API in which business logic should be put 15:14:41 <tcohen> this issue is related to point (1) on th elink 15:15:15 <tcohen> I've already expressed that my personal POV is that we should keep the current way of using DBIC 15:15:44 <tcohen> i.e. using it inside modules, that can be unit tested 15:16:03 <fridolin> +1 for UT 15:16:27 <gmcharlt> hmm 15:16:41 <gmcharlt> I think that point perhaps could be clarified - may I go through a somewhat lengthly example? 15:17:12 <tcohen> please 15:17:24 <ashimema> go for it 15:17:42 <gmcharlt> OK, so when we are talking about business logic, there are at least two ways in can manifest itself 15:17:53 <fridolin> point 1 : i think since there are aleardy modules nammed nearly like a db table (ie C4/Borrowers.pm), I'll vote for a) 15:18:06 <gmcharlt> umm, may I please have the floor 15:18:19 <fridolin> yep sorry 15:18:22 <tcohen> :) 15:18:48 <gmcharlt> so, one way business logic can manifest is as attributes of the underlying entities that we're dealing with 15:18:56 <gmcharlt> so take, for example, item records 15:19:27 <gmcharlt> one way of modeling an item record is to follow the DB schema closely 15:19:42 <gmcharlt> on the assumption that we've done a reasonably good job with our relational model 15:19:50 <gmcharlt> which, in many places, we have 15:19:53 <gmcharlt> in others, we haven't 15:19:59 <gmcharlt> but let's stick with items for now 15:20:38 <gmcharlt> one thing we could say is that, OK, a Koha::Schema::Result::Item is a decent base class for an item record 15:21:02 <gmcharlt> but that it doesn't /exactly/ give everything we need to ask of any item record 15:21:33 <gmcharlt> and the big example is ->effective_itemtype 15:22:02 <gmcharlt> there's a bit of business logic there, as of course the effective item type for circ polices depends on the item-level_itypes system preference 15:22:40 <gmcharlt> but it's somethat that can be well-encapsulated as (effectively) an attribute of K::S::R::Item 15:22:57 <gmcharlt> and there's a lot going for that approach 15:23:30 <gmcharlt> 1. it is definitely testable -- there's nothing stopping unit tests from being written on K::S::R::* objects 15:24:04 <gmcharlt> 2. it gives a *single* place to ask the question - "what is the effective item type of an item?" 15:24:17 <gmcharlt> so in that respect, I quite like it 15:24:28 <gmcharlt> next, I'm going to move on to an example where things are less clear-cut 15:24:33 <gmcharlt> namely, bibs 15:25:05 <gmcharlt> so, as everybody here knows, you need to look at both biblio and biblioitems to get all of the info regarding a bib entity 15:25:36 <gmcharlt> (and of course, that represents drift from Koha's original quasi-FRBR model, but that's in the past, water under the bridge, etc.) 15:26:04 <gmcharlt> but the upshot - this is an example where the current relational data model is flawed for our current purposes 15:26:41 <gmcharlt> as if you start from a K::S::R::Biblio object ... you have to go through some convulations to get at everything you might want to ask of a bib record object 15:27:06 <gmcharlt> now you /could/ do things like add K::S::R::Biblio->marc to grab the MARC record 15:27:15 <gmcharlt> and so on... but that's stretching things 15:27:42 <gmcharlt> and it's vulnerabile to cases where we would want to redo the data structure 15:27:53 <gmcharlt> (which, we probably want to do in the long run) 15:28:42 <gmcharlt> so there, I think the situation calls for (say) a Koha::Biblio object that can be *composed* of (say) a K::S::R::Biblio and a K::S::R::bibioitems object 15:29:17 <gmcharlt> but if we right such a class ... it would be useful to delegate as much to the underlying K::S::R attribute methods as possible 15:29:43 <gmcharlt> partcularly when we want to start traveling to related objects like items 15:30:02 <gmcharlt> so, now I'm going to move on to my third (and final) example 15:30:31 <gmcharlt> and that's business logic that pretty much doesn't naturally look like an attribute of an object 15:30:58 <gmcharlt> for example: the question of calculating the due date of a new loan 15:31:13 <gmcharlt> now you can envision procedural code 15:31:26 <gmcharlt> e.g., my $due_date = Koha::Loans::get_due_date($patron, $item) 15:31:33 <gmcharlt> or a more OO approach 15:31:46 <gmcharlt> my $loan = Koha::Loans->new($patron, $item) 15:32:00 <gmcharlt> where, in effect, the constructor of that object does things like calculate the due date 15:32:19 <gmcharlt> (and throw exceptions if the loan is against policy) 15:33:01 <gmcharlt> but however you do it, the upshot is that it wouldn't be a good idea to add lots of methods to Koha::S::R::Issue to try to do all of this 15:33:04 <gmcharlt> but rather, build a new class 15:33:09 <gmcharlt> (or build procedural code) 15:33:29 <gmcharlt> so what I've done so far is provided three examples of the sort of business logic we have to deal with 15:33:39 <cait> thx gmcharlt - that#s really helpful 15:33:49 <gmcharlt> next, I'm going to talk about testing 15:34:40 <gmcharlt> first, I submit that it doesn't matter for the prupose of being able to test things whether code exists as a K::S::R method or as a K::Object method to be reachable for testing 15:34:43 <gmcharlt> either way, it can be tested 15:35:05 <gmcharlt> IOW, I'm sayng that how we decide the DBIC usage questions need not be inlfuenced all that much by testing concerns 15:35:20 <gmcharlt> as we have both db-dependent and non-db-dependent ways of testing 15:35:26 * tcohen agrees 15:35:51 <gmcharlt> so as far as my current thinking on the overall matter is concerned 15:36:48 <gmcharlt> 1. I dislike adding layers of classes needlessly -- in particular, if a K::S::R object represents an entity adequately, I see no reason not to use it as a base class or a very thinly-encapsulated entity 15:37:11 <gmcharlt> e.g., K::S::R::Item->effective_itemtype is a good example 15:38:12 <gmcharlt> 2. Not *everything* can be done that way -- e.g., I would call for a Koha::Biblio class to be written that (among other things) contains K::S::R::Biblio and K::S::R::Biblioitems objects, and makes some effort to hide the details of those tables 15:38:26 <gmcharlt> as we KNOW that those tables are eventually going to significantly change 15:39:05 <gmcharlt> so the upshot: on question #1, I fall firmly in the middle 15:39:29 <gmcharlt> now we may want to provide some syntactic sugar to the reduce the typing to instantiant a K::S::R object 15:40:34 <gmcharlt> but for areas where our data model is good -- I think a lot can be done by adding attribute methods to the underlying K::S::R classes 15:41:16 <gmcharlt> and (to answer in part a question that cait had asked me) borrowers is an example where the database schema mostly matches how we view the objects 15:41:45 <gmcharlt> so I think that for now K::S::R::Borrowers would be a good base class 15:42:21 <gmcharlt> now that's not to say that we couldn't have a Koha::Borrowers, but for now it shoudl delegate/pass along as much as psosible to K::S::R::Borrowers 15:42:38 <cait> hm so we would put something there that pulls information from ohter tables as well? 15:42:47 <cait> sorry i am still trying to wrap my mind around it :) 15:42:59 <gmcharlt> well, K::S::R via DBIC does that for us for free 15:43:02 <tcohen> welcome ribasushi 15:43:15 <cait> so it would get the extended patron attributes etc. 15:43:16 <gmcharlt> at least in the cases where the underlying tables have FK relationsips 15:43:33 <cait> ok thx :) 15:43:50 <gmcharlt> as far as our quesinon #6 is concerned -- I'm not a fan of a Koha::Object and/or Koha::Object::Set as a universal base class 15:43:53 <gmcharlt> too much weight 15:44:06 <gmcharlt> if anything should be a universal base class, I'd go for Class::Accessor 15:44:14 <gmcharlt> and thus ends my monologue for now 15:44:45 <gmcharlt> but to summarize: I'm suggesting a hybrid approach, one of whose big advantages is that it will allow us to continue to switch thigns over to using DBIC incrementally 15:45:21 * gmcharlt goes under his desk to look for his hands, which have fallen off 15:45:49 <tcohen> other opinions please? 15:45:50 * cait helps searching... hard to put a hand back on with no hands... 15:47:09 <Joubu> gmcharlt: how do implement set/list? 15:47:12 <ashimema> I think gmcharlt described it pretty succinctly really. 15:47:13 <Joubu> you* 15:48:19 <gmcharlt> Joubu: one moment before I answer you, want to check something 15:50:05 <Joubu> rangi: around? if I remember correctly you was not fond of extending K::S::R into "Koha::Object" 15:50:38 <gmcharlt> Joubu: so for now, my view is to use DBIC::ResultSet where possible 15:51:06 <tcohen> gmcharlt: even on front end stuff like the .pl scripts¿ 15:51:23 <cait> Joubu: 3 am in nz i think :( 15:51:28 <ColinC> Yes it makes for clear robust code 15:51:30 <gmcharlt> provide constructures where needed so that you can (say) take a Koha::S::R::Biblio and initiatialize a Koha::Biblio object from it 15:51:49 <gmcharlt> and uses plain old arrays and hashes for sets ;) 15:52:06 * jcamins agrees with gmcharlt on everything that gmcharlt said. 15:52:35 <gmcharlt> and as far as the scripts are concerned... yeah, I personally have no problem fetching data directly using DBIC in them 15:52:36 <Joubu> ok, my fieling is the same, except I would create a Koha::Biblios or Koha::Biblio::Set 15:53:06 <gmcharlt> but the key thing is this: the scripts should be focused on GETTING the objects, but they wouldn't be doing much if any business logic 15:53:34 <gmcharlt> and where possible, would be passing the K::S::R and K::* objects directly to the template so that the TT code can take care of fetching attributes for display 15:53:59 <Joubu> gmcharlt: do you have a poc to show us? A class with search+crud? 15:54:33 <gmcharlt> Joubu: I disagree in the general case -- i.e., I wouldn't want to see a Koha::Vendor::Set or the like 15:54:44 <gmcharlt> however, catalog searches are a special case 15:55:00 <tcohen> patron searches? 15:55:13 <gmcharlt> i.e., I could certainly see a Koha::CatalogSearchResultSet class 15:55:19 <Joubu> "passing the K::S::R and K::* objects directly to the template" => to me, we should not manipulate both. Only Koha::* 15:55:29 <Joubu> manipulate in pl 15:55:43 <ColinC> No need for a Set object you just have a set of object much as ResultSet itself is a set of objs 15:56:32 <gmcharlt> Joubu: I think our main difference of opinion is that I think K::S::R objects should be used when they suffice, and that Koha::* classes should be created when they are necessary, but that we can do both and be happy 15:57:02 <Joubu> gmcharlt: I just want consistency, I would like to create all objects using the same way 15:57:05 <gmcharlt> from the POV of the templates, either way it's going to look like [% object.attribute %] anyway 15:57:24 <Joubu> my $patron = Koha::Patron->new($patron_info)->insert 15:58:05 <gmcharlt> Joubu: I want consistent *interfaces* for accessors -- I care much less about the base class 15:58:07 <ColinC> From the callers vire both should look the same 15:58:19 <Joubu> and @patrons = Koha::PatronSet->search(name => 'smith') 15:58:26 <Joubu> s/PatronSet/WhatYouWant 15:59:39 <gmcharlt> Joubu: I don't want to (or want anybody) to have to write 100+ sets of boilerplate objects to do that when K::S::R can handle that for most objects 15:59:40 <gmcharlt> HOWEVER 16:00:05 <tcohen> I don't like the hybrid approach: when does it suffice to use K::S::R? 16:00:06 <gmcharlt> there might be the possibility of some AUTOLOAD magic 16:00:21 <gmcharlt> tcohen: items, for now 16:00:26 <gmcharlt> but to continue my thought 16:01:23 <gmcharlt> there might be the possibility of some AUTOLOAD magic or the like so that we can chop out the "Schema::Result" part of the names to use the DBIC classes directly 16:01:38 <gmcharlt> and where needed, e.g., Koha::Biblio, override when something that's hand-constructed 16:02:05 <Joubu> (11518 (effective_itemtype) still NSO, for 6months, and we didn't get enough thoughts) 16:03:24 <cait> i think we becasue we needed ot have this discussion - hopefully the bugs stuck now can then start moving forward 16:04:03 <Joubu> I really would like to see how you (everybody) would like to write code in pl file. It's clear to me, but I didn't get ideas/thoughts from others. Don't you have a POC/try somewhere to show? 16:04:47 <Joubu> I think it's the first question to consider 16:05:40 <Joubu> When we answered, we should try to have a consistent, simple, easy to use and flexible API 16:07:33 <Joubu> hum, ping? 16:07:34 <wahanui> Using deft allegory, the authors have provided an insightful and intuitive explanation of one of Unix's most venerable networking utilities. http://www.amazon.com/Story-about-Reading-Railroad-Books/dp/0448421658 16:08:04 <tcohen> i'm here 16:08:05 <ColinC> been using dbic and its nice to have a consitent reliable approach where we need classes that span tables like biblio the api should be as similar as possible 16:08:10 <cait> i think the dual approach makes sense tome, but it might be hard to judge at times which route is better 16:09:06 <tcohen> i think in the long run we will have a better model, which makes DBIC classes represent our objects better 16:09:10 <tcohen> BUT 16:09:21 <tcohen> we will still need to put business logic on top of that 16:09:56 <tcohen> and in that case, extending Koha::Schema::ResultSet::* should be the way to go 16:10:24 <tcohen> we could think Koha::Biblio makes sense "only" because of a bad underlying model 16:11:23 <tcohen> if we can rely on DBIC for everything 16:12:12 <gmcharlt> right - my inclination is to have the RDBMS model match the desired class structure where possible 16:12:32 <gmcharlt> but knowing that it it won't be always possible 16:13:19 <gmcharlt> as far as Joubu's question is concerned - well, current DBIC use in scripts like admin/categorie.pl isn't actually horrid, but I would grant that it's certainly verbose 16:13:35 <gmcharlt> the trick would be improving that without burying the DBIC classes udner too many layers 16:14:08 <tcohen> by *too many* you mean one, right? 16:14:50 <gmcharlt> yep, if we can avoid it :) 16:15:35 <Joubu> it's for the lisiblity, maintenability 16:15:52 <Joubu> s/lisibility/readability 16:16:58 <Joubu> Does nobody want to write some code? 16:17:05 <Joubu> just some lines here? 16:18:26 <tcohen> any more comments/opinions? 16:20:21 <gmcharlt> Joubu: I'm not sure you get to make demands on anybody's time but your own -- but I think I have some ideas on how to implemnt some concision 16:20:27 <tcohen> ok, i think we have exhausted the discussion for now. I'll try to sumarize on the wiki Joubu set for this topic 16:20:36 <gmcharlt> in the meantime, I've got more utf8 test cases to write today 16:20:50 <tcohen> :-D 16:21:18 <tcohen> gmcharlt: do u have some sample test we can take a look at? 16:21:19 <Joubu> gmcharlt: I just would like to know how continue to develop 16:21:38 <gmcharlt> utf8 test cases first, then we'll see 16:21:49 <Joubu> gmcharlt: I had some tries, but they seem to be bad. 16:22:06 <tcohen> Joubu: you're talking about using DBIC? 16:22:35 <Joubu> gmcharlt: I don't ask a lot. Just how instanciate an object, save it. And possibly a search 16:23:04 <gmcharlt> Joubu: I know; I'll get to it, but I'm not making promises as to when 16:23:10 <Joubu> I need to unfreeze this situation 16:23:15 <Joubu> s/I/We 16:23:22 <gmcharlt> I don't disagree 16:23:34 <gmcharlt> I think the discussion today has been helpful 16:23:48 <gmcharlt> and should get even more help when the NZers weigh in 16:24:01 <gmcharlt> IOW, I think we're making progress, as slow as it may seem 16:24:15 <gmcharlt> Joubu++ # keeping at it 16:25:34 <tcohen> ok, i think the meeting concluded 16:25:49 <Joubu> There are tries on 12830, 12896, 8007, 10363 16:25:56 <tcohen> some more thoughts will be expressed later 16:26:09 <Joubu> so all approches is bad, and I will try something else 16:26:11 <tcohen> and we will work on this in person, during the hackfest 16:26:40 <tcohen> I encourage people that disagrees with a specific dev approach, to make it explicit 16:26:41 <Joubu> or 1 is good, and it would be good to know :) 16:26:46 <tcohen> how it should be done 16:26:54 <cait> i think the hackfest will be a chance and we can post the code for others to look at 16:26:59 <Joubu> http://www.gliffy.com/go/publish/6160409 16:27:02 <tcohen> because otherwise it discourages participation 16:27:16 <gmcharlt> Joubu: on a completely different topic, I just now learned of a code2bib French mailing list... is it like code4lib? 16:27:19 <Joubu> for information, maybe not a lot of people is aware of that. 16:27:56 <Joubu> gmcharlt: what the name of the ml? 16:27:58 <Joubu> is* 16:28:05 <gmcharlt> Joubu: code2bib 16:28:20 <tcohen> ok, thanks every1 16:28:25 <tcohen> #endmeeting