[whimsy] branch master updated: Fix monitoring to always use symbolic keys Check if public json log file has changed before copying Unit test improvement

[se...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sebb pushed a commit to branch master
in repository https://git-dual.apache.org/repos/asf/whimsy.git

The following commit(s) were added to refs/heads/master by this push:
       new  c3cc337   Fix monitoring to always use symbolic keys Check if public json log file has changed before copying Unit test improvement
c3cc337 is described below

commit c3cc337b8222cc465361f3001b92b77f6d7cc146
Author: Sebb <se...@apache.org>
AuthorDate: Wed Apr 6 23:29:58 2016 +0100

    Fix monitoring to always use symbolic keys
    Check if public json log file has changed before copying
    Unit test improvement
---
 www/status/monitor.rb              | 23 ++++++++++++++++++-----
 www/status/monitors/public_json.rb | 23 +++++++++++++----------
 www/status/monitors/system.rb      | 11 ++++++-----
 www/status/monitors/unit_test.rb   | 17 ++++++++++++++---
 4 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/www/status/monitor.rb b/www/status/monitor.rb
index 8be70f9..16d1ad0 100644
--- a/www/status/monitor.rb
+++ b/www/status/monitor.rb
@@ -1,8 +1,20 @@
 #
 # Overall monitor class is responsible for loading and running each
 # monitor in the `monitors` directory, collecting and normalizing the
-# results and outputting it as JSON.
+# results and outputting it as JSON for use in the GUI.
 #
+# The previous status is passed to the monitor on the next run so it can
+# determine when to take non-idempotent actions such as sending a message
+#
+# The monitors are called frequently, so the return status built up by
+# a monitor should be easily comparable with the input status. This means
+# using the same key and value formats (at least for monitors that need to
+# compare them).
+#
+# Although both string and symbolic keys can be used in hashes, the syntax
+# for symbolic keys is rather neater, so we use symbolic keys throughout.
+# This means that the JSON file is parsed into a hash using symbolic keys,
+# and any variables used as keys need to be converted to symbols.
 
 require 'json'
 require 'time'
@@ -21,8 +33,8 @@ class Monitor
       mtime = File.exist?(status_file) ? File.mtime(status_file) : Time.at(0)
       file.flock(File::LOCK_EX)
 
-      # fetch previous status
-      baseline = JSON.parse(file.read) rescue {}
+      # fetch previous status (using symbolic keys)
+      baseline = JSON.parse(file.read, {symbolize_names: true}) rescue {}
       baseline[:data] = {} unless baseline[:data].instance_of? Hash
 
       # If status was updated while waiting for the lock, use the new status
@@ -39,7 +51,7 @@ class Monitor
         threads << Thread.new do
           begin
             # invoke method to determine current status
-            previous = baseline[:data][method.to_s] || {mtime: Time.at(0)}
+            previous = baseline[:data][method.to_sym] || {mtime: Time.at(0)}
             status = Monitor.send(method, previous) || previous
 
             # convert non-hashes in proper statuses
@@ -116,7 +128,8 @@ class Monitor
       status[:level] ||= 'danger'
     end
 
-    # normalize time
+    # normalize time 
+    # If the called monitor wants to compare status hashes it should store the correct format
     if status[:mtime].instance_of? Time
       status[:mtime] = status[:mtime].gmtime.iso8601
     end
diff --git a/www/status/monitors/public_json.rb b/www/status/monitors/public_json.rb
index 9a7be03..35076c8 100644
--- a/www/status/monitors/public_json.rb
+++ b/www/status/monitors/public_json.rb
@@ -3,6 +3,7 @@
 #
 
 require 'fileutils'
+require 'time'
 
 def Monitor.public_json(previous_status)
   danger_period = 86_400 # one day
@@ -18,14 +19,14 @@ def Monitor.public_json(previous_status)
   status = {}
 
   Dir[logs].each do |log|
-    name = File.basename(log).sub('public-', '')
+    name = File.basename(log).sub('public-', '').to_sym
 
     begin
       status[name] = {
         href: "../logs/#{File.basename(log)}",
-        mtime: File.mtime(log)
+        mtime: File.mtime(log).gmtime.iso8601, # to agree with normalise
+        level: 'success' # to agree with normalise
       }
-
       contents = File.read(log, encoding: Encoding::UTF_8)
 
       # Ignore Wunderbar logging for normal messages (may occur multiple times)
@@ -81,13 +82,15 @@ def Monitor.public_json(previous_status)
       }
     end
 
-    # Save a copy of the log
-    # append the severity so can track more problems
-    lvl = status[name][:level] 
-    if lvl and lvl != 'info'
-      name = File.basename(log)
-      FileUtils.copy log, File.join(archive, name + '.' + lvl),
-        preserve: true
+    # Has there been a change since the last check?
+    if status[name] != previous_status[:data][name]
+      lvl = status[name][:level] 
+      if lvl and lvl != 'info' # was there a problem?
+        # Save a copy of the log; append the severity so can track more problems
+        name = File.basename(log)
+        # temporarily allow the date stamp to be updated so we can see if the file is copied mulitple times
+        FileUtils.copy log, File.join(archive, name + '.' + lvl) #, preserve: true
+      end
     end
   end
 
diff --git a/www/status/monitors/system.rb b/www/status/monitors/system.rb
index 34465e0..67a77ae 100644
--- a/www/status/monitors/system.rb
+++ b/www/status/monitors/system.rb
@@ -6,16 +6,17 @@
 require 'time'
 
 def Monitor.system(previous_status)
-  name='puppet'
+  name=:puppet
   status = {}
   status[name] = {
-    mtime: Time.now.gmtime.iso8601,
-    command: 'service puppet status'
+    command: 'service puppet status',
   }
   begin
     puppet = `service puppet status`.strip
-    if puppet.include? ' * agent is running'
-      status[name].merge! level: 'warning', title: puppet
+    if puppet.include? '* agent is running'
+      status[name].merge! level: 'success', data: puppet
+    else
+      status[name].merge! level: 'warning', data: puppet
     end
     rescue Exception => e
       status[name] = {
diff --git a/www/status/monitors/unit_test.rb b/www/status/monitors/unit_test.rb
index b16361d..ce36b92 100644
--- a/www/status/monitors/unit_test.rb
+++ b/www/status/monitors/unit_test.rb
@@ -6,8 +6,19 @@ require 'json'
 # the same string is used to extract the sample and call the method
 def runtest(method_name)
   status_file = File.expand_path('../../status.json', __FILE__)
-  baseline = JSON.parse(File.read(status_file)) rescue {}
+  baseline = JSON.parse(File.read(status_file),{symbolize_names: true}) rescue {}
   baseline[:data] = {} unless baseline[:data].instance_of? Hash
-  previous = baseline[:data][method_name] || {mtime: Time.at(0)}
-  puts JSON.pretty_generate(Monitor.send(method_name, previous))
+  previous = baseline[:data][method_name.to_sym] || {mtime: Time.at(0)}
+  response = Monitor.send(method_name, previous)
+  if response == previous
+    puts "No change in response"
+  elsif response[:data] and response[:data] == previous[:data]
+    # main class adds a trailer after the data
+    puts "No change in response data"
+  else
+    puts "Reponse differs:"
+    puts previous
+    puts response
+  end
+  puts JSON.pretty_generate(response)
 end

-- 
To stop receiving notification emails like this one, please contact
['"commits@whimsical.apache.org" <co...@whimsical.apache.org>'].