Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #9508 - extract the triggering and waiting for sub plans from bulk actions #99

Closed
wants to merge 2 commits into from

Conversation

iNecas
Copy link
Member

@iNecas iNecas commented Feb 23, 2015

Bulk actions is just one of the use-cases for using sub-plans. This patch
extract the common code to be used for triggering external tasks in general.

The usage is as follows: one needs to inherit it's action from the
Actions::ActionWithSubPlans and override the
create_sub_plans method. The create_sub_plans will be executed in run
phase and the rest of the action will make sure that the execution of the
action will wait till the sub plan is done. It also tracks the relation
between parent and child tasks, as it already did in bulk actions.

An example code could look like this (taken from real Katello actions):

class SyncAsSubPlan < Actions::ActionWithSubPlans
 input_format do
   param :id, Integer
 end

  def plan(repository)
   plan_self(:id => repository.id)
 end

  def create_sub_plans
   trigger(Repository::Sync, ::Katello::Repository.find(input[:id]))
 end

end

…ulk actions

Bulk actions is just one of the use-cases for using sub-plans. This
patch extract the common code to be used for triggering external tasks
in general.

The usage is as follows: one needs to inherit it's action
from the ``Actions::ActionWithSubPlans`` and override the
``create_sub_plans`` method. The ``create_sub_plans`` will be executed
in run phase and the rest of the action will make sure that the
execution of the action will wait till the sub plan is done. It also
tracks the relation between parent and child tasks, as it already did
in bulk actions.

An example code could look like this (taken from real Katello actions):

    class SyncAsSubPlan < Actions::ActionWithSubPlans
      input_format do
        param :id, Integer
      end

      def plan(repository)
        plan_self(:id => repository.id)
      end

      def create_sub_plans
        trigger(Repository::Sync, ::Katello::Repository.find(input[:id]))
      end
    end
@iNecas
Copy link
Member Author

iNecas commented Feb 25, 2015

@bbuckingham I've added the support for the locks of sub-plans to not collide with the parent's

@bbuckingham
Copy link
Member

@iNecas, I tested this PR out using a combination of foreman+katello+fusor to see the behavior of the fusor deploy action. That action creates a set of subplan actions to sync a set of repos. The result was successful (no lock conflicts or other errors). Nice work!


SubPlanFinished = Algebrick.type do
fields! :execution_plan_id => String,
:success => type { variants TrueClass, FalseClass }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bbuckingham
Copy link
Member

@iNecas, Is there a target for getting this merged in and built? Right now, I am using a checkout to work with, but would like to get to gem/rpm soon. We also need to see about getting it in to the downstream. Thanks!!

@iNecas
Copy link
Member Author

iNecas commented Mar 10, 2015

@bbuckingham getting back to this, will try to address the concerns from the review (+ some preparation for the rollbacks, that the locks relate a bit), the updated PR should be ready tomorrow, with the expected release later this week. Would be friday ok with you?

@bbuckingham
Copy link
Member

@iNecas, Sounds good. Thanks for the update and effort!

@iNecas
Copy link
Member Author

iNecas commented Mar 13, 2015

The updated PR is available at #104 (with dependent PR at Dynflow/dynflow#145), closing this PR in favor of these two. The advantage is we don't need to use the Thread.current for passing the task id info + we have the native support for that in Dynlfow (so that one can even link to the sub plans from dynflow console).

@bbuckingham could you please test the new PR against your code? Nothing should be changed from the usage point of view.

@iNecas iNecas closed this Mar 13, 2015
@bbuckingham
Copy link
Member

@iNecas, Sure, I'll run a clean test using the code in the updated PRs. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants