On Thu, Feb 4, 2016 at 12:55 PM, sebb <[email protected]> wrote:
> On 4 February 2016 at 15:31, Sam Ruby <[email protected]> wrote:
>> Commit 60f24d0cab030cf6d64ce75c23d05b1ac33bdda7:
>> if all hosts are down, give up
>>
>> Branch: refs/heads/master
>> Author: Sam Ruby <[email protected]>
>> Committer: Sam Ruby <[email protected]>
>> Pusher: rubys <[email protected]>
>>
>> ------------------------------------------------------------
>> lib/whimsy/asf/ldap.rb | ++ --
>> ------------------------------------------------------------
>> 4 changes: 2 additions, 2 deletions.
>> ------------------------------------------------------------
>>
>>
>> diff --git a/lib/whimsy/asf/ldap.rb b/lib/whimsy/asf/ldap.rb
>> index 0376c64..f73646b 100644
>> --- a/lib/whimsy/asf/ldap.rb
>> +++ b/lib/whimsy/asf/ldap.rb
>> @@ -73,7 +73,7 @@ def self.puppet_ldapservers
>>
>> # connect to LDAP
>> def self.connect
>> - loop do
>> + host.length.times do
>
> -1
>
> The original code works fine.
>
> The new loop tries to loop some 37 times (depending on the size of the
> host name) and then fails with
>
> iteration reached an end (StopIteration)
>
> I think the original code works rather better.
Can you explain how this works for me?
What I see is a "loop do...end" with the only exit being on a success,
and the code at the end of this method being unreachable.
I will say that whether there are 4 hosts or 50 hosts, trying exactly
37 times (depending on the length of an unrelated string) seems kinda
weird.
And failing with a StopIteration is less desirable than producing a
clean message, such as "Failed to connect to any LDAP host". I can
see the advantage of raising an exception over returning nil, but I
would prefer a more meaningful message.
- Sam Ruby
>> host = next_host
>> Wunderbar.info "Connecting to LDAP server: #{host}"
>>
>> @@ -97,8 +97,8 @@ def self.connect
>> Wunderbar.warn "Error connecting to LDAP server #{host}: " +
>> re.message + " (continuing)"
>> end
>> -
>> end
>> +
>> Wunderbar.error "Failed to connect to any LDAP host"
>> return nil
>> end
