Comment 6 for bug 1779920

Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Cesar. I made a pass through the code and have a few minor comments:

1. A number of INFO-level debug logs added to Trigger.pm should be removed or set to ->debug (or -internal). Ditto a new log in Validator.pm. At INFO, the Trigger log would be especially noisy.

2. In AutoRenew.pm a session connect call for open-ils.trigger is made without a corresponding disconnect. It looks like the purpose is to fire-and-forget the trigger autocreate call, so connecting serves no real purpose here. AppSession->create(...) instead of ->connect ensures the API calls will be spread among the existing trigger back-ends. Or just ->disconnect to cleanup. Or see AppUtils comment below.

3. In CircIsAutoRenewable, the trigger autocreate call is wrapped in a connect/disconnect which serves no purpose (but adds overhead) for single-call API requests.

Also note there's an AppUtils function for creating events: create_events_for_hook(...) -- with a fire-and-forget option.

4. Minor nit:

my $userObj = $AppUtils->simplereq('open-ils.cstore', 'open-ils.cstore.direct.actor.user.retrieve', $userid);

vs.

# Has better logging, easier to read.
my $userObj = new_editor()->retrieve_actor_user($userid);