We now pre-calculate the UTC timestamps of the day boundaries of the local
timezone, and use these to bucket the reports in the DB. This saves us
from having to rely on MySQL specific DB functions. This also allows us
to properly calculate the day boundaries across DST changes, rather than
relying on a single UTC offset for all day boundaries.
While we're at it, we're also consolidating by_interval, and
within_daily_run_history, since by_interval was only used by
within_daily_run_history, and provided unused flexibility.
Paired-with: Jesse Wolfe <je...@puppetlabs.com>
Paired-with: Nick Lewis <n...@puppetlabs.com>
Paired-with: Paul Berry <p...@puppetlabs.com>
Signed-off-by: Jacob Helwig <ja...@puppetlabs.com>
---
Local-branch: tickets/next/4403-fix-dst-handling
app/models/status.rb | 54 +++++----
spec/models/status_spec.rb | 300 ++++++++++++++++++++++++++++++++++++++------
2 files changed, 293 insertions(+), 61 deletions(-)
diff --git a/app/models/status.rb b/app/models/status.rb
index 9a8d3c9..e7afbaa 100644
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -11,43 +11,53 @@ class Status
# Returns an array of Statuses by date for either a :node, or :nodes or all
nodes in the system.
#
# Options:
- # * :node => Node to return Statuses for.
+ # * :node => Node to return Statuses for.
# * :nodes => Nodes to return Statuses for.
- # * :start => Start Time of the range to query.
- # * :limit => Limit the number of records to return.
- def self.by_interval(options={})
+ def self.within_daily_run_history(options={})
return [] if options[:nodes] && options[:nodes].empty?
- # WARNING: This uses the local server time, regardless of what is set in
the Rails config.
- # This should be changed once we have a user-friendly settings file, or
can get the browser
- # time zone to this method.
- offset = Time.zone.now.utc_offset
- offset_timestamp = "UNIX_TIMESTAMP(time) + #{offset}"
- date = "DATE(FROM_UNIXTIME(#{offset_timestamp}))"
+ last_day = Time.zone.now + 1.day # Last full day to include
(ignores time).
+ limit = SETTINGS.daily_run_history_length # Limit the number of days to
return (includes "last_day").
+
+ utc_date_boundaries = get_utc_boundaries_ending(last_day, limit + 1)
+ newest_accepatable_data = utc_date_boundaries.first
+
+ boundary_groupings = "CASE\n"
+ utc_date_boundaries.each do |boundary|
+ # We use the '%Y-%m-%d %H:%M:%S %z' strftime to get something parse-able
+ # by Time.zone.parse and lexically sortable.
+ boundary_groupings << "WHEN time >= '#{boundary.utc.to_s(:db)}' THEN
'#{boundary.strftime("%Y-%m-%d %H:%M:%S %z")}'\n"
+ end
+ boundary_groupings << "ELSE null\n"
+ boundary_groupings << "END"
sql = <<-SQL
SELECT
- COUNT(*) as total,
+ COUNT(*) as total,
SUM(CASE status when "unchanged" then 1 else 0 end) as unchanged,
- SUM(CASE status when "changed" then 1 else 0 end) as changed,
- SUM(CASE status when "failed" then 1 else 0 end) as failed,
- #{date} as start
+ SUM(CASE status when "changed" then 1 else 0 end) as changed,
+ SUM(CASE status when "failed" then 1 else 0 end) as failed,
+ #{boundary_groupings} as start
FROM reports
SQL
- sql << "WHERE kind = 'apply' "
- sql << "AND time >= \"#{options[:start].getutc.to_s(:db)}\"\n" if
options[:start]
- sql << "AND node_id = #{options[:node].id} " if options[:node]
+ sql << "WHERE kind = 'apply'\n"
+ sql << "AND time < \"#{newest_accepatable_data.utc.to_s(:db)}\"\n"
+ sql << "AND time >= \"#{utc_date_boundaries.last.utc.to_s(:db)}\"\n"
+ sql << "AND node_id = #{options[:node].id}\n" if
options[:node]
sql << "AND node_id IN (#{options[:nodes].map(&:id).join(',')})\n" if
options[:nodes].present?
- sql << "GROUP BY #{date}\n"
- sql << "ORDER BY time ASC\n"
- sql << "LIMIT #{options[:limit]}" if options[:limit]
+ sql << "GROUP BY start\n"
+ sql << "ORDER BY start ASC\n"
+ sql << "LIMIT #{limit}\n"
return execute(sql)
end
- def self.within_daily_run_history(options={})
- self.by_interval( options.merge( :start =>
SETTINGS.daily_run_history_length.days.ago, :limit =>
SETTINGS.daily_run_history_length ) )
+ def self.get_utc_boundaries_ending(date, num_days)
+ (0..(num_days-1)).collect do |offset|
+ x = date - offset.days
+ Time.zone.local(x.year, x.month, x.day, 0, 0, 0)
+ end
end
def self.runtime
diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb
index 22ed3a8..080dbc5 100644
--- a/spec/models/status_spec.rb
+++ b/spec/models/status_spec.rb
@@ -3,77 +3,299 @@ require File.expand_path(File.dirname(__FILE__) +
'/../spec_helper')
describe Status do
include ReportSupport
+ describe ".get_utc_boundaries_ending" do
+ it "should not skip days near a daylight savings time boundary" do
+ Time.zone = 'Pacific Time (US & Canada)'
+
+ Status.get_utc_boundaries_ending(Time.zone.parse("2011-03-14 00:30:00
-0700"), 4).should == [
+ Time.zone.parse("Mon, 14 Mar 2011 00:00:00 PDT -07:00"),
+ Time.zone.parse("Sun, 13 Mar 2011 00:00:00 PST -08:00"),
+ Time.zone.parse("Sat, 12 Mar 2011 00:00:00 PST -08:00"),
+ Time.zone.parse("Fri, 11 Mar 2011 00:00:00 PST -08:00"),
+ ]
+ end
+
+ it "should not consider DST when in a timezone without it" do
+ Time.zone = 'Tokyo'
+
+ Status.get_utc_boundaries_ending(Time.zone.parse("2011-03-14 00:30:00
+0900"), 4).should == [
+ Time.zone.parse("Mon, 14 Mar 2011 00:00:00 +09:00"),
+ Time.zone.parse("Sun, 13 Mar 2011 00:00:00 +09:00"),
+ Time.zone.parse("Sat, 12 Mar 2011 00:00:00 +09:00"),
+ Time.zone.parse("Fri, 11 Mar 2011 00:00:00 +09:00"),
+ ]
+ end
+ end
+
describe ".by_interval" do
- context "when the interval is 1 day" do
+ describe "when the local timezone is one that uses DST" do
before :each do
Time.zone = 'Pacific Time (US & Canada)'
+ end
- time = Time.zone.parse("2009-11-12 00:01 PST")
- Report.generate!(:time => time)
+ it "should handle crossing the DST start boundary" do
+ [
+ "2011-03-14 12:00 PDT",
+ "2011-03-14 00:00 PDT",
+ "2011-03-13 03:00 PDT",
+ "2011-03-13 01:00 PST",
+ "2011-03-12 00:00 PST",
+ "2011-03-11 00:00 PST",
+ ].each do |date_string|
+ time = Time.zone.parse(date_string)
+ Report.generate!(:time => time)
+ end
- time = Time.zone.parse("2009-11-11 23:59 PST")
- Report.generate!(:time => time)
+
ActiveSupport::TimeZone.any_instance.stubs(:now).returns(Time.zone.parse("2011-03-14
00:30:00 -0700"))
+ SETTINGS.stubs(:daily_run_history_length).returns(4)
- time = Time.zone.parse("2009-11-10 23:59 PST")
- Report.generate!(:time => time)
- Report.generate!(:time => time - 10)
+ Status.within_daily_run_history.map{|s| [s.start, s.total]}.
+ should == [
+ [Time.zone.parse("Fri, 11 Mar 2011 00:00:00 PST -08:00"), 1],
+ [Time.zone.parse("Sat, 12 Mar 2011 00:00:00 PST -08:00"), 1],
+ [Time.zone.parse("Sun, 13 Mar 2011 00:00:00 PST -08:00"), 2],
+ [Time.zone.parse("Mon, 14 Mar 2011 00:00:00 PDT -07:00"), 2]
+ ]
end
- it "should return reports for the correct day only" do
- Status.by_interval(:limit => 1, :start => Time.zone.parse("2009-11-11
00:00 PST")).
- map(&:start).should == [Time.zone.parse("2009-11-11 00:00 PST")]
+ it "should handle crossing the DST end boundary" do
+ [
+ "2010-11-08 01:00 PST",
+ "2010-11-07 03:00 PST",
+ "2010-11-07 02:00 PST",
+ "2010-11-07 00:00 PDT",
+ "2010-11-06 00:00 PDT",
+ ].each do |date_string|
+ time = Time.zone.parse(date_string)
+ Report.generate!(:time => time)
+ end
- Status.by_interval(:limit => 1, :start => Time.zone.parse("2009-11-12
00:00 PST")).
- map(&:start).should == [Time.zone.parse("2009-11-12 00:00 PST")]
+
ActiveSupport::TimeZone.any_instance.stubs(:now).returns(Time.zone.parse("2010-11-08
00:00:00 -0800"))
+ SETTINGS.stubs(:daily_run_history_length).returns(3)
- Status.by_interval(:limit => 1, :start => Time.zone.parse("2009-11-10
00:00 PST")).map(&:total).should == [2]
+ Status.within_daily_run_history.map{|s| [s.start, s.total]}.
+ should == [
+ [Time.zone.parse("Sat, 06 Nov 2010 00:00:00 PDT -07:00"), 1],
+ [Time.zone.parse("Sun, 07 Nov 2010 00:00:00 PDT -07:00"), 3],
+ [Time.zone.parse("Mon, 08 Nov 2010 00:00:00 PST -08:00"), 1]
+ ]
end
- it "should return reports after the start time" do
- Status.by_interval(:start => Time.zone.parse("2009-11-11 00:00 PST")).
- map(&:start).should == ["2009-11-11 00:00 PST", "2009-11-12 00:00
PST"].map {|time| Time.zone.parse time}
+ it "should handle crossing both the DST start and end boundaries at the
same time" do
+ [
+ # DST end boundary
+ "2010-11-08 01:00 PST",
+ "2010-11-07 02:01 PST",
+ "2010-11-07 01:59 PDT",
+ "2010-11-07 00:00 PDT",
+ "2010-11-06 00:00 PDT",
+ # DST start boundary
+ "2011-03-14 00:00 PDT",
+ "2011-03-13 03:00 PDT",
+ "2011-03-13 01:59 PST",
+ "2011-03-13 00:00 PST",
+ "2011-03-12 00:00 PST",
+ ].each do |date_string|
+ time = Time.zone.parse(date_string)
+ Report.generate!(:time => time)
+ end
+
+
ActiveSupport::TimeZone.any_instance.stubs(:now).returns(Time.zone.parse("2011-03-18
00:00:00 -0800"))
+ SETTINGS.stubs(:daily_run_history_length).returns(135)
+
+ Status.within_daily_run_history.map{|s| [s.start, s.total]}.
+ should == [
+ [Time.zone.parse("2010-11-06 00:00 PDT"), 1],
+ [Time.zone.parse("2010-11-07 00:00 PDT"), 3],
+ [Time.zone.parse("2010-11-08 00:00 PST"), 1],
+ [Time.zone.parse("2011-03-12 00:00 PST"), 1],
+ [Time.zone.parse("2011-03-13 00:00 PST"), 3],
+ [Time.zone.parse("2011-03-14 00:00 PDT"), 1]
+ ]
+ end
+
+ it "should only return reports within the given window" do
+ [
+ "2009-11-12 12:00 PST",
+ "2009-11-12 00:01 PST",
+ "2009-11-11 23:59 PST",
+ "2009-11-10 23:59 PST",
+ "2009-11-10 23:49 PST",
+ "2009-11-09 12:00 PST"
+ ].each do |date_string|
+ time = Time.zone.parse(date_string)
+ Report.generate!(:time => time)
+ end
+
+
ActiveSupport::TimeZone.any_instance.stubs(:now).returns(Time.zone.parse("2009-11-11
00:00 PST"))
+ SETTINGS.stubs(:daily_run_history_length).returns(2)
+
+ Status.within_daily_run_history.map{|s| [s.start, s.total]}.
+ should == [
+ [Time.zone.parse("2009-11-10 00:00 PST"), 2],
+ [Time.zone.parse("2009-11-11 00:00 PST"), 1],
+ ]
end
it "should not count inspect reports" do
+ [
+ "2009-11-12 12:00 PST",
+ "2009-11-12 00:01 PST",
+ "2009-11-11 23:59 PST",
+ "2009-11-10 23:59 PST",
+ "2009-11-10 23:49 PST",
+ "2009-11-09 12:00 PST"
+ ].each do |date_string|
+ time = Time.zone.parse(date_string)
+ Report.generate!(:time => time)
+ end
Report.generate(:time => Time.zone.parse("2009-11-10 12:00 PST"),
:kind => "inspect")
- Status.by_interval(:limit => 1, :start => Time.zone.parse("2009-11-10
00:00 PST")).map(&:total).should == [2]
+
ActiveSupport::TimeZone.any_instance.stubs(:now).returns(Time.zone.parse("2009-11-10
00:00 PST"))
+ SETTINGS.stubs(:daily_run_history_length).returns(1)
+
+ Status.within_daily_run_history.map{|s| [s.start, s.total]}.
+ should == [
+ [Time.zone.parse("2009-11-10 00:00 PST"), 2]
+ ]
end
end
- end
- describe ".within_daily_run_history" do
- context "when the daily_run_history_length is 1 day" do
+ describe "when the local timezone is one that does not use DST" do
before :each do
- time = Time.zone.now.beginning_of_day + 1.hours
- Report.generate!(:time => time)
+ Time.zone = 'Tokyo'
+ end
- time = Time.zone.now.beginning_of_day - 1.hours
- Report.generate!(:time => time)
+ it "should handle crossing the DST start boundary" do
+ [
+ # JST
+ "2011-03-14 12:00 +09:00",
+ "2011-03-14 00:00 +09:00",
+ "2011-03-13 03:00 +09:00",
+ "2011-03-13 01:00 +09:00",
+ "2011-03-12 00:00 +09:00",
+ "2011-03-11 00:00 +09:00",
+ ].each do |date_string|
+ time = Time.zone.parse(date_string)
+ Report.generate!(:time => time)
+ end
- SETTINGS.stubs(:daily_run_history_length).returns(1)
+
ActiveSupport::TimeZone.any_instance.stubs(:now).returns(Time.zone.parse("2011-03-14
00:30:00 +0900"))
+ SETTINGS.stubs(:daily_run_history_length).returns(4)
+
+ Status.within_daily_run_history.map{|s| [s.start, s.total]}.
+ should == [
+ [Time.zone.parse("Fri, 11 Mar 2011 00:00:00 +09:00"), 1],
+ [Time.zone.parse("Sat, 12 Mar 2011 00:00:00 +09:00"), 1],
+ [Time.zone.parse("Sun, 13 Mar 2011 00:00:00 +09:00"), 2],
+ [Time.zone.parse("Mon, 14 Mar 2011 00:00:00 +09:00"), 2]
+ ]
end
- it "should return reports for the correct day only" do
- Status.within_daily_run_history.first.total.should == 1
+ it "should handle crossing the DST end boundary" do
+ [
+ # JST
+ "2010-11-08 01:00 +09:00",
+ "2010-11-07 03:00 +09:00",
+ "2010-11-07 02:00 +09:00",
+ "2010-11-07 00:00 +09:00",
+ "2010-11-06 00:00 +09:00",
+ ].each do |date_string|
+ time = Time.zone.parse(date_string)
+ Report.generate!(:time => time)
+ end
+
+
ActiveSupport::TimeZone.any_instance.stubs(:now).returns(Time.zone.parse("2010-11-08
00:00:00 +0900"))
+ SETTINGS.stubs(:daily_run_history_length).returns(3)
+
+ Status.within_daily_run_history.map{|s| [s.start, s.total]}.
+ should == [
+ [Time.zone.parse("Sat, 06 Nov 2010 00:00:00 +09:00"), 1],
+ [Time.zone.parse("Sun, 07 Nov 2010 00:00:00 +09:00"), 3],
+ [Time.zone.parse("Mon, 08 Nov 2010 00:00:00 +09:00"), 1]
+ ]
end
- end
- context "when the daily_run_history_length is 0 days" do
- before :each do
- time = Time.zone.now.beginning_of_day + 1.hours
- Report.generate!(:time => time)
+ it "should handle crossing both the DST start and end boundaries at the
same time" do
+ [
+ # Non-existent DST end boundary in JST
+ "2010-11-08 01:00 +09:00",
+ "2010-11-07 02:01 +09:00",
+ "2010-11-07 01:59 +09:00",
+ "2010-11-07 00:00 +09:00",
+ "2010-11-06 00:00 +09:00",
+ # Non-existent DST start boundary in JST
+ "2011-03-14 00:00 +09:00",
+ "2011-03-13 03:00 +09:00",
+ "2011-03-13 01:59 +09:00",
+ "2011-03-13 00:00 +09:00",
+ "2011-03-12 00:00 +09:00",
+ ].each do |date_string|
+ time = Time.zone.parse(date_string)
+ Report.generate!(:time => time)
+ end
+
+
ActiveSupport::TimeZone.any_instance.stubs(:now).returns(Time.zone.parse("2011-03-18
00:00:00 +0900"))
+ SETTINGS.stubs(:daily_run_history_length).returns(135)
- time = Time.zone.now.beginning_of_day - 1.hours
- Report.generate!(:time => time)
+ Status.within_daily_run_history.map{|s| [s.start, s.total]}.
+ should == [
+ [Time.zone.parse("2010-11-06 00:00 +09:00"), 1],
+ [Time.zone.parse("2010-11-07 00:00 +09:00"), 3],
+ [Time.zone.parse("2010-11-08 00:00 +09:00"), 1],
+ [Time.zone.parse("2011-03-12 00:00 +09:00"), 1],
+ [Time.zone.parse("2011-03-13 00:00 +09:00"), 3],
+ [Time.zone.parse("2011-03-14 00:00 +09:00"), 1]
+ ]
+ end
+
+ it "should only return reports within the given window" do
+ [
+ # JST
+ "2009-11-12 12:00 +09:00",
+ "2009-11-12 00:01 +09:00",
+ "2009-11-11 23:59 +09:00",
+ "2009-11-10 23:59 +09:00",
+ "2009-11-10 23:49 +09:00",
+ "2009-11-09 12:00 +09:00"
+ ].each do |date_string|
+ time = Time.zone.parse(date_string)
+ Report.generate!(:time => time)
+ end
- SETTINGS.stubs(:daily_run_history_length).returns(0)
+
ActiveSupport::TimeZone.any_instance.stubs(:now).returns(Time.zone.parse("2009-11-11
00:00 JST"))
+ SETTINGS.stubs(:daily_run_history_length).returns(2)
+
+ Status.within_daily_run_history.map{|s| [s.start, s.total]}.
+ should == [
+ [Time.zone.parse("2009-11-10 00:00 +09:00"), 2],
+ [Time.zone.parse("2009-11-11 00:00 +09:00"), 1],
+ ]
end
- it "should not return any history" do
- Status.within_daily_run_history.should == []
+ it "should not count inspect reports" do
+ [
+ # JST
+ "2009-11-12 12:00 +09:00",
+ "2009-11-12 00:01 +09:00",
+ "2009-11-11 23:59 +09:00",
+ "2009-11-10 23:59 +09:00",
+ "2009-11-10 23:49 +09:00",
+ "2009-11-09 12:00 +09:00"
+ ].each do |date_string|
+ time = Time.zone.parse(date_string)
+ Report.generate!(:time => time)
+ end
+ Report.generate(:time => Time.zone.parse("2009-11-10 12:00 +09:00"),
:kind => "inspect")
+
+
ActiveSupport::TimeZone.any_instance.stubs(:now).returns(Time.zone.parse("2009-11-10
00:00 +09:00"))
+ SETTINGS.stubs(:daily_run_history_length).returns(1)
+
+ Status.within_daily_run_history.map{|s| [s.start, s.total]}.
+ should == [
+ [Time.zone.parse("2009-11-10 00:00 +09:00"), 2]
+ ]
end
end
end
-
end
--
1.7.4.1
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
