Just a few comments inline (I haven't applied/tested it though) [email protected] wrote: > From: martyntaylor <[email protected]> > > > diff --git a/src/app/models/instance_observer.rb > b/src/app/models/instance_observer.rb > new file mode 100644 > index 0000000..81e75ea > --- /dev/null > +++ b/src/app/models/instance_observer.rb > @@ -0,0 +1,22 @@ > +class InstanceObserver < ActiveRecord::Observer > + > + def after_save(an_instance) > + if an_instance.changed? > + change = an_instance.changes['state'] > + if change > + update_timestamps(change[0], change[1], an_instance) > + end > + end > + end > + > + def update_timestamps(state_from, state_to, an_instance) > + if state_to == Instance::STATE_RUNNING > + an_instance.time_last_start = Time.now > + elsif state_from == Instance::STATE_RUNNING && state_to == > Instance::STATE_STOPPED > + an_instance.acc_run_time = an_instance.acc_run_time + (Time.now - > an_instance.time_last_start) > + end > + end > We should make sure that there aren't any hidden edge cases here -- instances going from STATE_RUNNING to _some other state_ and then later to STATE_STOPPED, for example, wouldn't be tracked properly here, if that's a valid state transition -- some providers might return some temporary "stopping" or "stop pending" state which might then later change to stopped. I'm not sure how the various provider FSM diagrams handle stopping, but we need to handle all transitions from running states to stopped states here. > + > +end > + > +InstanceObserver.instance > \ No newline at end of file > diff --git a/src/app/models/task.rb b/src/app/models/task.rb > index 6cb907e..7cf7347 100644 > --- a/src/app/models/task.rb > +++ b/src/app/models/task.rb > @@ -21,6 +21,7 @@ > > class Task < ActiveRecord::Base > belongs_to :task_target, :polymorphic => true > + > # InstanceTask association > belongs_to :instance, :class_name => "Instance", > :foreign_key => "task_target_id" > @@ -37,8 +38,28 @@ class Task < ActiveRecord::Base > COMPLETED_STATES = [STATE_FINISHED, STATE_FAILED, STATE_CANCELED] > WORKING_STATES = [STATE_QUEUED, STATE_RUNNING, STATE_PAUSED] > > + # - Failure Codes - > + > + # Failures that happen in the provider > + PROVIDER_OVER_QUOTA = "PROVIDER_OVER_QUOTA" > + PROVIDER_FAILED_TASK = "PROVIDER_FAILED_TASK" > + PROVIDER_CONNECTION_FAILURE = "PROVIDER_CONNECTION_FAILURE" > + > + # Failures that happen in the aggregator > + AGGREGATOR_OVER_POOL_QUOTA = "AGGREGATOR_OVER_POOL_QUOTA" > + AGGREGATOR_OVER_USER_QUOTA = "AGGREGATOR_OVER_USER_QUOTA" > We don't need the last one above. There are currently two quota types defined in the aggregator: 1) quota on a pool (shown above) limits amount of resource use by a user/users for a given pool (so this would effectively be a 'user quota', but it's enforced at the pool level) 2) quota on a back end account. If an account is over quota, this will prevent the scheduler from sending it any more 'start instance' requests. If all otherwise-matching accounts are over quota, this will result in a 'no match found' error state as you define below > + AGGREGATOR_NO_MATCHING_PROVIDER = "AGGREGATOR_NO_MATCHING_PROVIDER" > This should ptobably be AGGREGATOR_NO_MATCHING_PROVIDER_ACCOUNT -- since it's the accounts within the providers that are the basic unit of resource matching > + > + PROVIDER_FAILURE_CODES = [PROVIDER_OVER_QUOTA, PROVIDER_FAILED_TASK, > PROVIDER_CONNECTION_FAILURE] > + AGGREGATOR_FAILURE_CODES = [AGGREGATOR_OVER_POOL_QUOTA, > AGGREGATOR_OVER_USER_QUOTA, AGGREGATOR_NO_MATCHING_PROVIDER] > + > + FAILURE_CODES = PROVIDER_FAILURE_CODES + AGGREGATOR_FAILURE_CODES + [nil] > + > + validates_inclusion_of :failure_code, > + :in => FAILURE_CODES > + > validates_inclusion_of :type, > - :in => %w( InstanceTask ) > + :in => %w( InstanceTask ) > > validates_inclusion_of :state, > :in => COMPLETED_STATES + WORKING_STATES > @@ -82,6 +103,7 @@ class Task < ActiveRecord::Base > def type_label > self.class.name[0..-5] > end > + > def task_obj > "" > end > diff --git a/src/app/models/task_observer.rb b/src/app/models/task_observer.rb > new file mode 100644 > index 0000000..2614519 > --- /dev/null > +++ b/src/app/models/task_observer.rb > @@ -0,0 +1,24 @@ > +class TaskObserver < ActiveRecord::Observer > + > + END_STATES = [ Task::STATE_CANCELED, Task::STATE_FAILED, > Task::STATE_FINISHED ] > + > + def after_save(a_task) > + if a_task.changed? > + change = a_task.changes['state'] > + if change > + update_timestamp(change[0], change[1], a_task) > + end > + end > + end > + > + def update_timestamp(state_from, state_to, a_task) > + if END_STATES.include?(state_to) > + a_task.time_ended = Time.now > + elsif state_to == Task::STATE_RUNNING > + a_task.time_started = Time.now > + end > + end > + > +end > + > +TaskObserver.instance > \ No newline at end of file >
_______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
