Quick refactoring recommendation:

Pull the lookup out into a helper

attr_reader :physical_proxies

@physical_proxies = { XXX => "proxyX", XXY => "proxyY", ... }

def proxy_lookup(virtal, ip)
  if virtual == "physical"
   physical_proxies[ip.split(/\./)[1]]
  elsif virtual == "openvz"
    some_other_code_returning_the_proxy
  end
end


Facter.add(:etproxy) do
 setcode do
   proxy_lookup(Facter.value(:virtual), Facter.value(:ipaddress))
 end
end

That's just off the cuff, you lose the semantics of all the naming which you
can either add in comments or by adding an intermediate lookup of the
location, then the proxy.  The more you pull it apart, the more you could
test it.

Also, since this looks like it is just a static lookup that would be the
same for each host, it would be better to implement it as an rvalue function
on the server, something like this:

require 'etproxy'

module Puppet::Parser::Functions
    newfunction(:proxy_lookup, :type => rvalue) do |args|
        ETProxy.lookup(lookupvar('virtual'), lookupvar('ipaddress'))
    end
end

http://reductivelabs.com/trac/puppet/wiki/WritingYourOwnFunctions

The ETProxy class and lookup are pretty much the code from above.

0.02
Andrew

On Mon, Jan 12, 2009 at 4:22 PM, Luke Kanies <l...@madstop.com> wrote:

>
> In general, ruby does not allow an explicit 'return' within blocks.
>
> Feel free to read up on the gory details, but basically, have the last
> statement in your block be the value you want to return, rather than
> an explicit return.
>
> On Jan 12, 2009, at 10:51 AM, Jeff Leggett wrote:
>
> >
> > If I do that, it blows up:
> >
> > [jlegg...@lxp6d11m8v190 facter]$ facter
> > /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter/etproxy.rb:36: unexpected
> > return (LocalJumpError)
> >        from /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter/util/
> > resolution.rb:117:in `call'
> >        from /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter/util/
> > resolution.rb:117:in `value'
> >        from /opt/etrade/p6/lib/ruby/1.8/timeout.rb:48:in `timeout'
> >        from /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter/util/
> > resolution.rb:115:in `value'
> >        from /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter/util/fact.rb:
> > 75:in `value'
> >        from /opt/etrade/p6/lib/ruby/site_ruby/1.8/rubygems/
> > custom_require.rb:27:in `inject'
> >        from /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter/util/fact.rb:
> > 71:in `each'
> >        from /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter/util/fact.rb:
> > 71:in `inject'
> >         ... 7 levels...
> >        from /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter/util/
> > collection.rb:103:in `to_hash'
> >        from /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter.rb:92:in
> > `send'
> >        from /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter.rb:92:in
> > `to_hash'
> >        from /opt/etrade/p6/bin/facter:121
> >
> >
> > On Jan 10, 3:39 am, "Ohad Levy" <ohadl...@gmail.com> wrote:
> >> You need to return a value, not to print it...
> >>
> >> try replacing put with return.
> >>
> >> Ohad
> >>
> >> On Sat, Jan 10, 2009 at 1:53 AM, Jeff Leggett
> >> <jeffrey.legg...@etrade.com>wrote:
> >>
> >>
> >>
> >>> OK Good to know, updated... So no error now but it still output the
> >>> facter value etproxy at the beginning of the facter outpout and
> >>> without an identifier:
> >>
> >>> [jlegg...@lxp6d11m8v190 facter]$ facter
> >>> 10.X.Y.Z:8080
> >>> architecture => i386
> >>> domain => etrade.com
> >>> facterversion => 1.5.2
> >>> fqdn => lxp6d11m8v190.etrade.com
> >>> hardwareisa => athlon
> >>> hardwaremodel => i686
> >>> <snipped>
> >>
> >>> I would expect to see a value for etproxy => 10.X.Y.Z:8080
> >>
> >>> here's the code:
> >>
> >>> Facter.add(:etproxy) do
> >>>  setcode do
> >>>     virtual = Facter.value(:virtual)
> >>>     if virtual = "physical"
> >>>        Alpharetta_proxy = "SNIPPED"
> >>>        Arlington_proxy = "SNIPPED"
> >>>        Austin_proxy = "SNIPPED"
> >>>        Manassas_proxy = "SNIPPED"
> >>>        Menlo_proxy = "SNIPPED"
> >>
> >>>        srcIP = Facter.value(:ipaddress)
> >>>        octets = srcIP.split( /\./ )
> >>>        case octets[1]
> >>>                <SNIPPED>
> >>>                else etproxy = Alpharetta_proxy
> >>>        end
> >>>        puts etproxy
> >>>    elsif virtual = "openvz"
> >>>       <other code>
> >>>    end
> >>>  end
> >>> end
> >>
> >>> On Jan 9, 4:59 am, "Ohad Levy" <ohadl...@gmail.com> wrote:
> >>>> Sorry, no need for the require 'facter' as you are already
> >>>> running inside
> >>>> it... (dahaa.) but if you want to try it out on irb or similar
> >>>> you need
> >>> it.
> >>
> >>>> Ohad
> >>
> >>>> On Fri, Jan 9, 2009 at 5:51 PM, Ohad Levy <ohadl...@gmail.com>
> >>>> wrote:
> >>>>> try in your code:
> >>
> >>>>> require 'facter'
> >>
> >>>>> Facter.value(:ipaddress) ( or Facter.ipaddress)
> >>
> >>>>> Cheers,
> >>>>> Ohad
> >>
> >>>>> On Fri, Jan 9, 2009 at 2:41 PM, Jeff Leggett <
> >>> jeffrey.legg...@etrade.com>wrote:
> >>
> >>>>>> That may be the problem - I call it by:
> >>
> >>>>>> ipaddr = `facter ipaddress`.chomp
> >>
> >>>>>> On Jan 8, 4:22 pm, James Turnbull <ja...@lovedthanlost.net>
> >>>>>> wrote:
> >>>>>>> -----BEGIN PGP SIGNED MESSAGE-----
> >>>>>>> Hash: SHA1
> >>
> >>>>>>> Jeff Leggett wrote:
> >>>>>>>> So I wrote a new fact to determine  clients proxy.  Work's
> >>>>>>>> great
> >>> when
> >>>>>>>> I call it via facter as a single:
> >>
> >>>>>>>> [jlegg...@lxp6d11m8v190 jleggett]$ facter etproxy
> >>>>>>>> 10.X.Y.Z:8080
> >>
> >>>>>>>> But when I call facter alone (for full output of all facts) I
> >>>>>>>> see
> >>> this
> >>>>>>>> at beginning:
> >>
> >>>>>>>> [jlegg...@lxp6d11m8v190 jleggett]$ facter
> >>>>>>>> /opt/etrade/p6/lib/ruby/site_ruby/1.8/facter/util/loader.rb:72:
> >>>>>>>> command not found: facter ipaddress
> >>>>>>>> 10.X.Y.Z:8080
> >>
> >>>>>>>> (I call fatcer ipaddress in my new fact - is that the problem?)
> >>
> >>>>>>>> Nothing here indicates I need to do something else:
> >>>>>>>> http://reductivelabs.com/trac/puppet/wiki/AddingFacts?
> >>
> >>>>>>>> What'd I do wrong?
> >>
> >>>>>>> Can you pastie your code?
> >>
> >>>>>>> Do you call facter ipaddress as a binary or use:
> >>
> >>>>>>> Facter.value(:ipaddress)
> >>
> >>>>>>> Regards
> >>
> >>>>>>> James Turnbull
> >>
> >>>>>>> - --
> >>>>>>> Author of:
> >>>>>>> * Pulling Strings with Puppet
> >>>>>>> (http://www.amazon.com/gp/product/1590599780/)
> >>>>>>> * Pro Nagios 2.0
> >>>>>>> (http://www.amazon.com/gp/product/1590596099/)
> >>>>>>> * Hardening Linux
> >>>>>>> (http://www.amazon.com/gp/product/1590594444/)
> >>
> >>>>>>> -----BEGIN PGP SIGNATURE-----
> >>>>>>> Version: GnuPG v1.4.7 (Darwin)
> >>>>>>> Comment: Using GnuPG with Mozilla -http://enigmail.mozdev.org
> >>
> >>>>>>> iD8DBQFJZm6e9hTGvAxC30ARAtT8AJwO6hah0YC0FWv3UbD7b+kos4Ku0gCeNsF7
> >>>>>>> /2fHE+dAc5PW+DWU6O+jhW4=
> >>>>>>> =hQBC
> >>>>>>> -----END PGP SIGNATURE------ Hide quoted text -
> >>
> >>>>>>> - Show quoted text -
> > >
>
>
> --
> The intelligent man finds almost everything ridiculous, the sensible
> man hardly anything. -- Johann Wolfgang von Goethe
> ---------------------------------------------------------------------
> Luke Kanies | http://reductivelabs.com | http://madstop.com
>
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com
To unsubscribe from this group, send email to 
puppet-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/puppet-users?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to