Yeah, I should probably extract that conditional into a method. I wanted to write it in such a way that someone looking at the diffs would have no doubt that this was just a mechanical merge between the two versions of the code. anyway, I expect that we'll probably drop support for the old bindings before too long, but I didn't want to pull the rug out from anyone who had gone to the trouble of getting it running.
On Sat, Sep 25, 2010 at 6:53 AM, Brice Figureau < [email protected]> wrote: > +1, with a couple of cosmetic comments: > > The following is used a lot throughout the patch: > + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd? > > This could be extracted in a method: > def use_legacy_rrd? > Puppet.features.rrd_legacy? && ! Puppet.features.rrd? > end > > small note: I would have used "and not" instead of "&& !" because I find > it more readable, but it's fine like this. > > The conditionals can also be removed easily by using polymorphism, if we > want to go this route :) > > On 25/09/10 02:36, Jesse Wolfe wrote: > > The rrd project has been shipping ruby bindings since 1.3.0, and the old > > rrdtool ruby library is no longer being maintained. > > > > This patch is based upon Davor Ocelic's submitted code, with the > > addition that I've added conditionals so it can still call the old > > rrdtool library if a modern rrd is not installed. > > > > Signed-off-by: Jesse Wolfe <[email protected]> > > --- > > lib/puppet/feature/base.rb | 3 +- > > lib/puppet/util/metric.rb | 47 > +++++++++++++++++++++++++++++++++++-------- > > 2 files changed, 40 insertions(+), 10 deletions(-) > > > > diff --git a/lib/puppet/feature/base.rb b/lib/puppet/feature/base.rb > > index c153fba..c983f5c 100644 > > --- a/lib/puppet/feature/base.rb > > +++ b/lib/puppet/feature/base.rb > > @@ -27,7 +27,8 @@ Puppet.features.add :diff, :libs => %w{diff/lcs > diff/lcs/hunk} > > Puppet.features.add(:augeas, :libs => ["augeas"]) > > > > # We have RRD available > > -Puppet.features.add(:rrd, :libs => ["RRDtool"]) > > +Puppet.features.add(:rrd_legacy, :libs => ["RRDtool"]) > > +Puppet.features.add(:rrd, :libs => ["RRD"]) > > > > # We have OpenSSL > > Puppet.features.add(:openssl, :libs => ["openssl"]) > > diff --git a/lib/puppet/util/metric.rb b/lib/puppet/util/metric.rb > > index 7e14a5f..0089847 100644 > > --- a/lib/puppet/util/metric.rb > > +++ b/lib/puppet/util/metric.rb > > @@ -31,9 +31,12 @@ class Puppet::Util::Metric > > > > start ||= Time.now.to_i - 5 > > > > - @rrd = RRDtool.new(self.path) > > args = [] > > > > + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd? > > + @rrd = RRDtool.new(self.path) > > + end > > + > > values.each { |value| > > # the 7200 is the heartbeat -- this means that any data that isn't > > # more frequently than every two hours gets thrown away > > @@ -42,14 +45,22 @@ class Puppet::Util::Metric > > args.push "RRA:AVERAGE:0.5:1:300" > > > > begin > > - @rrd.create( Puppet[:rrdinterval].to_i, start, args) > > + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd? > > + @rrd.create( Puppet[:rrdinterval].to_i, start, args) > > + else > > + RRD.create( self.path, '-s', Puppet[:rrdinterval].to_i.to_s, > '-b', start.to_i.to_s, *args) > > + end > > rescue => detail > > raise "Could not create RRD file #{path}: #{detail}" > > end > > end > > > > def dump > > - puts @rrd.info > > + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd? > > + puts @rrd.info > > + else > > + puts RRD.info self.path > > + end > > end > > > > def graph(range = nil) > > @@ -82,14 +93,26 @@ class Puppet::Util::Metric > > args << lines > > args.flatten! > > if range > > - args.push("--start",range[0],"--end",range[1]) > > + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd? > > + args.push("--start",range[0],"--end",range[1]) > > + else > > + > args.push("--start",range[0].to_i.to_s,"--end",range[1].to_i.to_s) > > + end > > else > > - args.push("--start", Time.now.to_i - time, "--end", > Time.now.to_i) > > + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd? > > + args.push("--start", Time.now.to_i - time, "--end", > Time.now.to_i) > > + else > > + args.push("--start", (Time.now.to_i - time).to_s, "--end", > Time.now.to_i.to_s) > > + end > > end > > > > begin > > #Puppet.warning "args = #{args}" > > - RRDtool.graph( args ) > > + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd? > > + RRDtool.graph( args ) > > + else > > + RRD.graph( *args ) > > + end > > rescue => detail > > Puppet.err "Failed to graph #{self.name}: #{detail}" > > end > > @@ -114,13 +137,15 @@ class Puppet::Util::Metric > > end > > > > def store(time) > > - unless Puppet.features.rrd? > > + unless Puppet.features.rrd? || Puppet.features.rrd_legacy? > > Puppet.warning "RRD library is missing; cannot store metrics" > > return > > end > > self.create(time - 5) unless FileTest.exists?(self.path) > > > > - @rrd ||= RRDtool.new(self.path) > > + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd? > > + @rrd ||= RRDtool.new(self.path) > > + end > > > > # XXX this is not terribly error-resistant > > args = [time] > > @@ -133,7 +158,11 @@ class Puppet::Util::Metric > > arg = args.join(":") > > template = temps.join(":") > > begin > > - @rrd.update( template, [ arg ] ) > > + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd? > > + @rrd.update( template, [ arg ] ) > > + else > > + RRD.update( self.path, '-t', template, arg ) > > + end > > #system("rrdtool updatev #{self.path} '#{arg}'") > > rescue => detail > > raise Puppet::Error, "Failed to update #{self.name}: #{detail}" > -- > Brice Figureau > My Blog: http://www.masterzen.fr/ > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]<puppet-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
