[OPEN-ILS-DEV] PATCH: Throw away bootstrap.conf and use opensrf_core.xml

[Mike Rylander]

On 6/28/07, Dan Scott <denials at gmail.com> wrote:
> On 27/06/07, Mike Rylander <mrylander at gmail.com> wrote:
> > On 6/27/07, Dan Scott <denials at gmail.com> wrote:
> > > Building on Nathan Eady's suggestion / intention from December
> >
> > Per discussion (and Dan's suggestion) in IRC, we're holding off on
> > this patch for further refinement of the in-module documentation (and
> > a DCO).
>
> Now featuring up-to-date perldoc _AND_ a DCO. Still not sure this is
> exactly what you want technically, though. The major limitation is
> that I've hardcoded /config/opensrf to appear as the 'bootstrap'
> section of config for compatibility with the expectations of the rest
> of OpenSRF. A broader patch would convert all of those other calls
> from config->bootstrap->blah to config->opensrf->blah and make the
> siblings of /config/opensrf visible in the config as well, as a more
> generally useful approach.
>
> My belief at this point however is that this patch serves the present
> goal of eliminating the need for bootsrap.conf; I can always expand
> this later.
>

I think the approach is fine ... it seems a very direct way of not
disrupting the API.  I do have a concern about part of
OpenSRF::Utils::Config::Section, though.  Specifically:

        foreach my $key (sort keys %$bootstrap) {
                $self->_sub_builder($key);
                if (ref($key) eq 'ARRAY') {
                        $self->$key([$bootstrap->{$key}]);
                }
                else {
                        $self->$key($bootstrap->{$key});
                }
        }

AFAICT the first branch of that if/else can't be executed (can't use a
ref as the key in a hash), and if it did, it would be wrapping an
array ref in another array ref.

The intention, I think, is to handle lists of elements coming from
OpenSRF::Utils::Config::load_config, such as when you have
<domains><domain>...</domain><domain>...</domain></domains>, but the
code isn't needed.

Much like the 'list:' qualifier syntax in the original module, this
module forcibly creates an array ref to store one or more values -- in
this case the syntax marker is a parent element instead of the key
prefix.  All we need to do is call _sub_builder on the $key and then
hand it the value to store.

Analysis of this chunk of code does expose one other deviation from
the existing API:  in the original module, an array ref was created
whether there were values to populate it or not

   $value = [split /\s*,\s*/, $value];

but the new code will pass undef instead of an empty array ref in that
case.  Currently, this could cause great consternation in
OpenSRF::Transport::SlimJabber::Inbound when it's time to disconnect
from the routers ... admittedly, not a big deal right now (the process
is presumably going away), but avoiding regressions is good, and doing
that in this case is easy.  We'll just set the value of
$bootstrap{$node->nodeName()} to an array ref inside load_config if
$child_state is 0 (first time through the loop).

I can make these changes, or just kick it back to you.  Just say the word.

--miker