Fixing #95. I had to redesign how events were triggered; the transaction now individually triggers each subscription, so that it has control in how to respond to failures. Eventually, this will lead the way to error handling within puppet, but for now, it just allows us to trigger every appropriate subscription, whether some have failed or not.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@998 980ebf18-57e1-0310-9a29-db15c13687c0

1 files changed, 29 insertions, 91 deletions

diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 0b1e88553..182401cb5 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -1,38 +1,27 @@
 # the class that actually walks our object/state tree, collects the changes,
 # and performs them
 
-# there are two directions of walking:
-#   - first we recurse down the tree and collect changes
-#   - then we walk back up the tree through 'refresh' after the changes
-
 require 'puppet'
 require 'puppet/statechange'
 
-#---------------------------------------------------------------
 module Puppet
 class Transaction
     attr_accessor :toplevel, :component, :objects
 
-    #---------------------------------------------------------------
     # a bit of a gross hack; a global list of objects that have failed to sync,
     # so that we can verify during later syncs that our dependencies haven't
     # failed
     def Transaction.init
         @@failures = Hash.new(0)
         Puppet::Metric.init
-        @@changed = []
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # for now, just store the changes for executing linearly
     # later, we might execute them as we receive them
     def change(change)
         @changes.push change
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # okay, here's the deal:
     # a given transaction maps directly to a component, and each transaction
     # will only ever receive changes from its respective component
@@ -67,7 +56,6 @@ class Transaction
                     # use an array, so that changes can return more than one
                     # event if they want
                     events = [change.forward].flatten.reject { |e| e.nil? }
-                    #@@changed.push change.state.parent
                 rescue => detail
                     change.state.err "change from %s to %s failed: %s" %
                         [change.state.is_to_s, change.state.should_to_s, detail]
@@ -94,60 +82,13 @@ class Transaction
         }.flatten.reject { |child|
             child.nil? # remove empties
         }
-#        events = @changes.collect { |change|
-#            if change.is_a?(Puppet::StateChange)
-#                change.transaction = self
-#                events = nil
-#                begin
-#                    # use an array, so that changes can return more than one
-#                    # event if they want
-#                    events = [change.forward].flatten.reject { |e| e.nil? }
-#                    #@@changed.push change.state.parent
-#                rescue => detail
-#                    change.state.err "change from %s to %s failed: %s" %
-#                        [change.state.is_to_s, change.state.should_to_s, detail]
-#                    #Puppet.err("%s failed: %s" % [change.to_s,detail])
-#                    if Puppet[:debug]
-#                        puts detail.backtrace
-#                    end
-#                    next
-#                    # FIXME this should support using onerror to determine
-#                    # behaviour; or more likely, the client calling us
-#                    # should do so
-#                end
-#
-#                # This is kinda lame, because it can result in the same
-#                # object being modified multiple times, but that's difficult
-#                # to avoid as long as we're syncing each state individually.
-#                change.state.parent.cache(:synced, now)
-#
-#                unless events.nil? or (events.is_a?(Array) and events.empty?)
-#                    change.changed = true
-#                end
-#                events
-#            else
-#                puts caller
-#                raise Puppet::DevError,
-#                    "Transactions cannot handle objects of type %s" % change.class
-#            end
-#        }.flatten.reject { |event|
-#            event.nil?
-#        }
 
         Puppet.debug "Finishing transaction %s with %s changes" %
             [self.object_id, count]
-        #@triggerevents = []
-        events.each { |event|
-            object = event.source
-            #Puppet::Event::Subscriptions.propagate(object, event, self)
-            object.propagate(event, self)
-        }
 
-        #events += @triggerevents
+        self.propagate(events)
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # this should only be called by a Puppet::Container object now
     # and it should only receive an array
     def initialize(objects)
@@ -165,26 +106,36 @@ class Transaction
         end
 
         @changes = []
+    end
+
+    # Respond to each of the events.  This method walks up the parent tree,
+    # triggering each parent in turn.  It's important that the transaction
+    # itself know whether a given subscription fails, so that it can respond
+    # appropriately (when we get to the point where we're responding to events).
+    def propagate(events)
+        events.each do |event|
+            source = event.source
+
+            while source
+                Puppet::Event::Subscription.trigger(source, event) do |sub|
+                    begin
+                        sub.trigger(self)
+                    rescue => detail
+                        sub.target.err "Failed to respond to %s: %s" % [event, detail]
+                        if Puppet[:debug]
+                            puts detail.backtrace
+                        end
+                    end
+                end
 
-        # change collection is in-band, and message generation is out-of-band
-        # of course, exception raising is also out-of-band
-        now = Time.now.to_i
-#        @changes = @objects.find_all { |child|
-#            child.scheduled?
-#        }.collect { |child|
-#            # these children are all Puppet::Type instances
-#            # not all of the children will return a change, and Containers
-#            # return transactions
-#            #ary = child.evaluate
-#            #ary
-#            child.evaluate
-#        }.flatten.reject { |child|
-#            child.nil? # remove empties
-#        }
+                # Reset the source if there's a parent obj
+                source = source.parent
+            end
+            #Puppet::Event::Subscriptions.propagate(object, event, self)
+        end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
+    # Roll all completed changes back.
     def rollback
         events = @changes.reverse.collect { |change|
             if change.is_a?(Puppet::StateChange)
@@ -196,7 +147,6 @@ class Transaction
                 #change.transaction = self
                 begin
                     change.backward
-                    #@@changed.push change.state.parent
                 rescue => detail
                     Puppet.err("%s rollback failed: %s" % [change,detail])
                     if Puppet[:debug]
@@ -220,30 +170,18 @@ class Transaction
             end
         }.flatten.reject { |e| e.nil? }
 
-        #@triggerevents = []
-        events.each { |event|
-            object = event.source
-            object.propagate(event, self)
-        }
-
-        #events += @triggerevents
+        self.propagate(events)
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     def triggered(object, method)
         @triggered[object][method] += 1
         #@triggerevents << ("%s_%sed" % [object.class.name.to_s, method.to_s]).intern
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     def triggered?(object, method)
         @triggered[object][method]
     end
-    #---------------------------------------------------------------
 end
 end
-#---------------------------------------------------------------
 
 # $Id$