On 4 February 2016 at 19:45, Sam Ruby <[email protected]> wrote: > 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.
loop/end works in conjunction with next_host which is an enum. enum.next generates StopIteration which is specifically caught by loop/end. http://ruby-doc.org/core-1.9.3/StopIteration.html > 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. It does it 37 or so times because host holds the host URL, and that is roughly its length. The host is the wrong variable to use. But so is the hosts array wrong, because that will always return the original number of hosts, even if some have already been used up and we are resuming the loop. The loop will then fall off the end of the enum, generating a StopIteration, which is not caught by the x.length.times do loop. > 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. It only *fails* with StopIteration with the updated code. That is because the loop count is too large, so next_host is called more times than there are hosts. With the previous code, next_host is also called one extra time, but the generated StopIteration is caught by the loop/do, as designed. http://ruby-doc.org/core-1.9.3/StopIteration.html Try defining single failing URL in your .whimsy file and you will see what I mean. > - 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
