11185: Merge branch 'master' into 11185-wb-disable-reuse
authorLucas Di Pentima <lucas@curoverse.com>
Tue, 2 May 2017 14:56:56 +0000 (11:56 -0300)
committerLucas Di Pentima <lucas@curoverse.com>
Tue, 2 May 2017 14:56:56 +0000 (11:56 -0300)
apps/workbench/app/controllers/container_requests_controller.rb
apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
apps/workbench/test/controllers/container_requests_controller_test.rb
services/api/test/fixtures/container_requests.yml

index ef7665b34d9f46dac52ac0c4c88f84c4127164ea..fd29cd3f7088b5ab2ca4011d7d6d8b9ab26b6182 100644 (file)
@@ -87,7 +87,26 @@ class ContainerRequestsController < ApplicationController
 
     @object = ContainerRequest.new
 
-    @object.command = src.command
+    # By default the copied CR won't be reusing containers, unless use_existing=true
+    # param is passed.
+    command = src.command
+    if params[:use_existing]
+      @object.use_existing = true
+      # Pass the correct argument to arvados-cwl-runner command.
+      if src.command[0] == 'arvados-cwl-runner'
+        command = src.command - ['--disable-reuse']
+        command.insert(1, '--enable-reuse')
+      end
+    else
+      @object.use_existing = false
+      # Pass the correct argument to arvados-cwl-runner command.
+      if src.command[0] == 'arvados-cwl-runner'
+        command = src.command - ['--enable-reuse']
+        command.insert(1, '--disable-reuse')
+      end
+    end
+
+    @object.command = command
     @object.container_image = src.container_image
     @object.cwd = src.cwd
     @object.description = src.description
@@ -100,7 +119,6 @@ class ContainerRequestsController < ApplicationController
     @object.runtime_constraints = src.runtime_constraints
     @object.scheduling_parameters = src.scheduling_parameters
     @object.state = 'Uncommitted'
-    @object.use_existing = false
 
     # set owner_uuid to that of source, provided it is a project and writable by current user
     current_project = Group.find(src.owner_uuid) rescue nil
index fd7953551729d722b8cff678feb73baadc81f7d2..049be759beed84778bf4bf35bb474f3e83e7a0cd 100644 (file)
@@ -1,10 +1,44 @@
 <% if @object.state == 'Final' %>
-  <%= link_to(copy_container_request_path('id' => @object.uuid),
-      class: 'btn btn-sm btn-primary',
-      title: 'Re-run',
-      data: {toggle: :tooltip, placement: :top}, title: 'This will make a copy and take you there. You can then make any needed changes and run it',
-      method: :post,
-      ) do %>
-    <i class="fa fa-fw fa-play"></i> Re-run
-  <% end %>
+<script type="application/javascript">
+  function reset_form_cr_reuse() {
+    $('#use_existing').removeAttr('checked');
+  }
+</script>
+
+  <%= link_to raw('<i class="fa fa-fw fa-play"></i> Re-run...'),
+      "#",
+      {class: 'btn btn-sm btn-primary', 'data-toggle' => 'modal',
+       'data-target' => '#clone-and-edit-modal-window',
+       title: 'This will make a copy and take you there. You can then make any needed changes and run it'}  %>
+
+<div id="clone-and-edit-modal-window" class="modal fade" role="dialog"
+     aria-labelledby="myModalLabel" aria-hidden="true">
+  <div class="modal-dialog">
+    <div class="modal-content">
+
+    <%= form_tag copy_container_request_path do |f| %>
+
+      <div class="modal-header">
+        <button type="button" class="close" onClick="reset_form_cr_reuse()" data-dismiss="modal" aria-hidden="true">&times;</button>
+        <div>
+          <div class="col-sm-6"> <h4 class="modal-title">Re-run container request</h4> </div>
+        </div>
+        <br/>
+      </div>
+
+      <div class="modal-body">
+              <%= check_box_tag(:use_existing, "true", false) %>
+              <%= label_tag(:use_existing, "Enable container reuse") %>
+      </div>
+
+      <div class="modal-footer">
+        <button class="btn btn-default" onClick="reset_form_cr_reuse()" data-dismiss="modal" aria-hidden="true">Cancel</button>
+        <button type="submit" class="btn btn-primary" name="container_request[state]" value="Uncommitted">Copy and edit inputs</button>
+      </div>
+
+    </div>
+    <% end %>
+  </div>
+</div>
+
 <% end %>
index 6f5a6daa100a83974526d5f3892ccbec4a491813..bd2f6beb6b45a6270825ba16eb34b2a0b00120ad 100644 (file)
@@ -38,23 +38,52 @@ class ContainerRequestsControllerTest < ActionController::TestCase
     get :show, {id: uuid}, session_for(:active)
     assert_response :success
 
-   assert_includes @response.body, "href=\"/container_requests/#{uuid}/copy\""
+   assert_includes @response.body, "action=\"/container_requests/#{uuid}/copy\""
   end
 
-  test "container request copy" do
-    completed_cr = api_fixture('container_requests')['completed']
-    post(:copy,
-         {
-           id: completed_cr['uuid']
-         },
-         session_for(:active))
-    assert_response 302
-    copied_cr = assigns(:object)
-    assert_not_nil copied_cr
-    assert_equal 'Uncommitted', copied_cr[:state]
-    assert_equal "Copy of #{completed_cr['name']}", copied_cr['name']
-    assert_equal completed_cr['cmd'], copied_cr['cmd']
-    assert_equal completed_cr['runtime_constraints']['ram'], copied_cr['runtime_constraints'][:ram]
+  [
+    ['completed', false, false],
+    ['completed', true, false],
+    ['completed-older', false, true],
+    ['completed-older', true, true],
+  ].each do |cr_fixture, reuse_enabled, uses_acr|
+    test "container request #{uses_acr ? '' : 'not'} using arvados-cwl-runner copy #{reuse_enabled ? 'with' : 'without'} reuse enabled" do
+      completed_cr = api_fixture('container_requests')[cr_fixture]
+      # Set up post request params
+      copy_params = {id: completed_cr['uuid']}
+      if reuse_enabled
+        copy_params.merge!({use_existing: true})
+      end
+      post(:copy, copy_params, session_for(:active))
+      assert_response 302
+      copied_cr = assigns(:object)
+      assert_not_nil copied_cr
+      assert_equal 'Uncommitted', copied_cr[:state]
+      assert_equal "Copy of #{completed_cr['name']}", copied_cr['name']
+      assert_equal completed_cr['cmd'], copied_cr['cmd']
+      assert_equal completed_cr['runtime_constraints']['ram'], copied_cr['runtime_constraints'][:ram]
+      if reuse_enabled
+        assert copied_cr[:use_existing]
+      else
+        refute copied_cr[:use_existing]
+      end
+      # If the CR's command is arvados-cwl-runner, the appropriate flag should
+      # be passed to it
+      if uses_acr
+        if reuse_enabled
+          # arvados-cwl-runner's default behavior is to enable reuse
+          assert_includes copied_cr['command'], 'arvados-cwl-runner'
+          assert_not_includes copied_cr['command'], '--disable-reuse'
+        else
+          assert_includes copied_cr['command'], 'arvados-cwl-runner'
+          assert_includes copied_cr['command'], '--disable-reuse'
+          assert_not_includes copied_cr['command'], '--enable-reuse'
+        end
+      else
+        # If no arvados-cwl-runner is being used, the command should be left alone
+        assert_equal completed_cr['command'], copied_cr['command']
+      end
+    end
   end
 
   [
index 76f59c29f87a3ebbad2672a42a5f1502883c2b13..d5bb47f6ff30e1bcae98944a47844b01f4d98c99 100644 (file)
@@ -109,7 +109,7 @@ completed-older:
   container_image: test
   cwd: test
   output_path: test
-  command: ["echo", "hello"]
+  command: ["arvados-cwl-runner", "echo", "hello"]
   container_uuid: zzzzz-dz642-compltcontainr2
   runtime_constraints:
     vcpus: 1