From 210af1cc7ede88914026fde078e45ef84c187a0c Mon Sep 17 00:00:00 2001
From: radhika <radhika@curoverse.com>
Date: Wed, 8 Feb 2017 19:16:57 -0500
Subject: [PATCH] 10903: use cancel with cascade to cancel jobs and pause
pipelines.
---
.../pipeline_instances_controller.rb | 9 ++++++
apps/workbench/app/models/job.rb | 2 +-
apps/workbench/app/models/job_work_unit.rb | 4 +++
.../workbench/app/models/pipeline_instance.rb | 4 +++
apps/workbench/app/models/work_unit.rb | 4 +++
.../_show_tab_buttons.html.erb | 5 +--
.../views/work_units/_show_component.html.erb | 7 ++--
apps/workbench/config/routes.rb | 1 +
.../test/integration/work_units_test.rb | 32 +++++++++++++++----
9 files changed, 55 insertions(+), 13 deletions(-)
diff --git a/apps/workbench/app/controllers/pipeline_instances_controller.rb b/apps/workbench/app/controllers/pipeline_instances_controller.rb
index d68c5795d0..a7b9142f0d 100644
--- a/apps/workbench/app/controllers/pipeline_instances_controller.rb
+++ b/apps/workbench/app/controllers/pipeline_instances_controller.rb
@@ -341,6 +341,15 @@ class PipelineInstancesController < ApplicationController
@unreadable_inputs_present
end
+ def cancel
+ @object.cancel
+ if params[:return_to]
+ redirect_to params[:return_to]
+ else
+ redirect_to @object
+ end
+ end
+
protected
def for_comparison v
if v.is_a? Hash or v.is_a? Array
diff --git a/apps/workbench/app/models/job.rb b/apps/workbench/app/models/job.rb
index 7bfed6d44b..346aef35d2 100644
--- a/apps/workbench/app/models/job.rb
+++ b/apps/workbench/app/models/job.rb
@@ -27,7 +27,7 @@ class Job < ArvadosBase
end
def cancel
- arvados_api_client.api "jobs/#{self.uuid}/", "cancel", {}
+ arvados_api_client.api "jobs/#{self.uuid}/", "cancel", {"cascade" => true}
end
def self.queue_size
diff --git a/apps/workbench/app/models/job_work_unit.rb b/apps/workbench/app/models/job_work_unit.rb
index a3f13f388c..5b1d1b7e35 100644
--- a/apps/workbench/app/models/job_work_unit.rb
+++ b/apps/workbench/app/models/job_work_unit.rb
@@ -81,6 +81,10 @@ class JobWorkUnit < ProxyWorkUnit
state_label.in? ["Queued", "Running"]
end
+ def confirm_cancellation
+ "All unfinished child jobs and pipelines will also be canceled, even if they are being used in another job or pipeline. Are you sure you want to cancel this job?"
+ end
+
def uri
uuid = get(:uuid)
"/jobs/#{uuid}"
diff --git a/apps/workbench/app/models/pipeline_instance.rb b/apps/workbench/app/models/pipeline_instance.rb
index e9fa04ab6d..b6e0ef17ae 100644
--- a/apps/workbench/app/models/pipeline_instance.rb
+++ b/apps/workbench/app/models/pipeline_instance.rb
@@ -136,6 +136,10 @@ class PipelineInstance < ArvadosBase
PipelineInstanceWorkUnit.new(self, label || self.name, self.uuid)
end
+ def cancel
+ arvados_api_client.api "pipeline_instances/#{self.uuid}/", "cancel", {"cascade" => true}
+ end
+
private
def components_map
diff --git a/apps/workbench/app/models/work_unit.rb b/apps/workbench/app/models/work_unit.rb
index dd4a706f9d..e10f8b7f5f 100644
--- a/apps/workbench/app/models/work_unit.rb
+++ b/apps/workbench/app/models/work_unit.rb
@@ -119,6 +119,10 @@ class WorkUnit
# returns true if this work unit can be canceled
end
+ def confirm_cancellation
+ # returns true if this work unit wants to use a confirmation for cancellation
+ end
+
def uri
# returns the uri for this work unit
end
diff --git a/apps/workbench/app/views/pipeline_instances/_show_tab_buttons.html.erb b/apps/workbench/app/views/pipeline_instances/_show_tab_buttons.html.erb
index 4ed27346b4..261f5a3c71 100644
--- a/apps/workbench/app/views/pipeline_instances/_show_tab_buttons.html.erb
+++ b/apps/workbench/app/views/pipeline_instances/_show_tab_buttons.html.erb
@@ -27,10 +27,11 @@
<% end %>
<% else %>
<% if @object.state.in? ['RunningOnClient', 'RunningOnServer'] %>
- <%= link_to(url_for('pipeline_instance[state]' => 'Paused'),
+ <%= link_to(cancel_pipeline_instance_path,
class: 'btn btn-primary run-pipeline-button',
title: 'Pause this pipeline',
- method: :patch
+ data: {confirm: 'All unfinished child jobs will be canceled, even if they are being used in another job or pipeline. Are you sure you want to pause this pipeline?'},
+ method: :post
) do %>
<i class="fa fa-fw fa-pause"></i> Pause
<% end %>
diff --git a/apps/workbench/app/views/work_units/_show_component.html.erb b/apps/workbench/app/views/work_units/_show_component.html.erb
index e8c4f1375f..ff9139221a 100644
--- a/apps/workbench/app/views/work_units/_show_component.html.erb
+++ b/apps/workbench/app/views/work_units/_show_component.html.erb
@@ -4,8 +4,8 @@
<div class="col-md-4">
<% if wu.is_paused? %>
<p>
- This <%= wu.title %> is paused. Children that are already running
- will continue to run, but no new processes will be submitted.
+ This <%= wu.title %> is paused. Children that were running
+ were cancelled and no new processes will be submitted.
</p>
<% end %>
@@ -23,9 +23,10 @@
</div>
<div class="col-md-2">
<% if wu.can_cancel? and @object.editable? %>
+ <% confirm = if wu.confirm_cancellation then {confirm: wu.confirm_cancellation} else {} end %>
<%= form_tag "#{wu.uri}/cancel", remote: true, style: "display:inline; padding-left: 1em" do |f| %>
<%= hidden_field_tag :return_to, url_for(@object) %>
- <%= button_tag "Cancel", {class: 'btn btn-xs btn-warning', id: "cancel-obj-button"} %>
+ <%= button_tag "Cancel", {class: 'btn btn-xs btn-warning', id: "cancel-obj-button", data: confirm} %>
<% end %>
<% end %>
</div>
diff --git a/apps/workbench/config/routes.rb b/apps/workbench/config/routes.rb
index 21cb7c40bc..0d5b8fca83 100644
--- a/apps/workbench/config/routes.rb
+++ b/apps/workbench/config/routes.rb
@@ -75,6 +75,7 @@ ArvadosWorkbench::Application.routes.draw do
get 'choose', on: :collection
end
resources :pipeline_instances do
+ post 'cancel', :on => :member
get 'compare', on: :collection
post 'copy', on: :member
end
diff --git a/apps/workbench/test/integration/work_units_test.rb b/apps/workbench/test/integration/work_units_test.rb
index 5b5848ee77..91b382d1bd 100644
--- a/apps/workbench/test/integration/work_units_test.rb
+++ b/apps/workbench/test/integration/work_units_test.rb
@@ -60,11 +60,11 @@ class WorkUnitsTest < ActionDispatch::IntegrationTest
end
[
- ['jobs', 'running_job_with_components', true],
- ['pipeline_instances', 'components_is_jobspec', false],
+ ['jobs', 'running_job_with_components', true, true],
+ ['pipeline_instances', 'components_is_jobspec', true, true],
['containers', 'running', false],
['container_requests', 'running', true],
- ].each do |type, fixture, cancelable|
+ ].each do |type, fixture, cancelable, confirm_cancellation|
test "cancel button for #{type}/#{fixture}" do
if cancelable
need_selenium 'to cancel'
@@ -74,14 +74,32 @@ class WorkUnitsTest < ActionDispatch::IntegrationTest
visit page_with_token "active", "/#{type}/#{obj['uuid']}"
assert_text 'created_at'
-
if cancelable
assert_text 'priority: 1' if type.include?('container')
- assert_selector 'button', text: 'Cancel'
- first('a,button', text: 'Cancel').click
+ if type.include?('pipeline')
+ assert_selector 'a', text: 'Pause'
+ first('a,link', text: 'Pause').click
+ else
+ assert_selector 'button', text: 'Cancel'
+ first('a,button', text: 'Cancel').click
+ end
+ if confirm_cancellation
+ alert = page.driver.browser.switch_to.alert
+ alert.accept
+ end
wait_for_ajax
end
- assert_text 'priority: 0' if cancelable and type.include?('container')
+
+ if type.include?('pipeline')
+ assert_selector 'a', text: 'Resume'
+ assert_no_selector 'a', text: 'Pause'
+ elsif type.include?('job')
+ assert_text 'Cancelled'
+ assert_text 'Paused' # this job has a pipeline child which was also cancelled
+ assert_no_selector 'button', text: 'Cancel'
+ elsif cancelable
+ assert_text 'priority: 0'
+ end
end
end
--
2.30.2