[OPEN-ILS-DEV] Problems with open-ils.ingest.full.biblio.record_list

[Dan Scott]

2009/4/8 Mike Rylander <mrylander at gmail.com>:
> On Wed, Apr 8, 2009 at 12:21 AM, Dan Scott <denials at gmail.com> wrote:
<snip>

>> I've fixed these issues in r12816 using a minimalist approach, but
>> it's not clear to me why the current implementation is as complex as
>> it is. ingest.full.biblio.record_list seems to be a copy of
>> ingest.full.biblio.record, with a bit of extra code to iterate over
>> the incoming list and to return a count of the records that were
>> successfully processed.
>
> They could certainly be coalesced into a single implementation.  The
> only important difference is null vs 0 on a single (or all) record
> error.  Calling code looks at the return value of both as true/false,
> so returning the bib id from the non-list version isn't needed.  That
> being said, it would not be hard to retain both existing return types
> based on $self->api_name.

Ah, the $self->api_name trick; that would be a good direction to go.
Thanks for the suggestion, Mike.

>>  Replacing it with an implementation that just
>> iterates over ingest.full.biblio.record for each element of the list
>> (below) works just as well, and (at least to my eyes) is easier to
>> read.
>>
>
> [snip implementation]
>
> For long lists of record ids, the extra round-tripping of a retrieval
> per record can cause significant slowdown.  There was once a version
> that was much like what you have there, but I rewrote it to do batch
> re-ingests more efficiently.

Oh, that makes perfect sense (of course). If we touch that area of
code again without refactoring it away entirely, we should probably
add a comment to that effect so that when Evergreen developer
archaelogists from the year 3000 dig into the code, they won't run
through the same rediscovery process. :)

-- 
Dan Scott
Laurentian University