theforeman / foreman_rh_cloud

a plugin to Foreman that generates and uploads reports to the Red Hat cloud
GNU General Public License v3.0
6 stars 32 forks source link

Schedule a random delay before execution #783

Closed ShimShtein closed 1 year ago

ShimShtein commented 1 year ago

Adding another option for randomizing task schedule. This correlates to option 1 in https://github.com/theforeman/foreman_rh_cloud/pull/781

The idea is to wait in the main task before starting the execution of the sub-tasks.

ShimShtein commented 1 year ago

This looks good, although I would prefer not hijacking the part action with the module. So instead of having a module, we'd have a new action class and a module, the new action would serve as the delay itself and the module would provide just a shortcut to insert the "blocking" action into the right place within the execution plan

If I understand you correctly, you would like to have a DelayAction that would do the delay, and then execute the actual code. It means that I would need to schedule the DelayAction and pass it the actual class as a parameter and I wasn't sure how I can pull it off with recurring action.

adamruzicka commented 1 year ago

have a DelayAction that would do the delay, and then execute the actual code

No, the delay action would only do the delay and that would be all. What I had in mind is really similar to what you have here, it just allows the top-level action to have its own #run.

It will be easier to explain with an example

module DelayedStart
  extend ActiveSupport::Concern

  def after_delay(delay = nil, &block)
    sequence do
      plan_action(DelayAction, delay: delay)
      concurrence(&block)
    end
  end
end

class DelayAction < ::Dynflow::Action
  def run
    # plan_event + suspend or finish
  end
end

module Async
  class GenerateAllReportsJob < ::Actions::EntryAction
    # ...
    include ForemanInventoryUpload::Async::DelayedStart

    def plan
      after_delay do
        # whatever
      end
    end
  end
end
ShimShtein commented 1 year ago

@adamruzicka now it looks much better, thanks for the suggestion!

ShimShtein commented 1 year ago

@adamruzicka fixed the nitpicks, ready for another round

Ron-Lavi commented 1 year ago

yes, setting up the environment and I will try to test it