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

[Mike Rylander]

On Wed, Apr 8, 2009 at 12:21 AM, Dan Scott <denials at gmail.com> wrote:
> In both trunk and rel_1_4, the implementation of
> open-ils.ingest.full.biblio.record_list seems to be broken; it
> immediately returns an undefined value because we're passing $rec
> (scalar value of @rec) to the bre search method. Changing this to a
> reference to the list fixes the problem.
>
> We were also returning a value consisting of the sum of the record IDs
> that were processed, rather than a count of the IDs.
>
> 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.

>  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.

-- 
Mike Rylander
 | VP, Research and Design
 | Equinox Software, Inc. / The Evergreen Experts
 | phone:  1-877-OPEN-ILS (673-6457)
 | email:  miker at esilibrary.com
 | web:  http://www.esilibrary.com