[ARVADOS] updated: 2.1.0-32-g5efbca217

Git user git at public.arvados.org
Fri Oct 23 01:02:27 UTC 2020


Summary of changes:
 .../controllers/container_requests_controller.rb   | 63 +++++++++++++++++-----
 .../app/controllers/work_units_controller.rb       |  1 +
 apps/workbench/app/helpers/application_helper.rb   |  5 ++
 apps/workbench/app/models/container_request.rb     | 20 +++++++
 .../_extra_tab_line_buttons.html.erb               | 47 +++++-----------
 .../views/container_requests/_show_inputs.html.erb | 18 +++----
 .../container_requests_controller_test.rb          | 23 ++++----
 apps/workbench/test/integration/work_units_test.rb |  4 +-
 8 files changed, 112 insertions(+), 69 deletions(-)

       via  5efbca2174eefd17f7b249a0662aeb30de4ec9c5 (commit)
       via  a71bf126349fba53bd0e90dc01dc17cfc656a7f4 (commit)
       via  9b77183d358c03dd74fced14bee920554e1cc94f (commit)
       via  6fab25e336eb334525c8a506183281a350772d1f (commit)
       via  d4dd59e26aa0874d32145dcc0d91cfd36ac6948c (commit)
       via  1ca30429c393325a091e3f7e30b36e204bbe2fca (commit)
       via  d9b10653c157a46b77bb21ff5bffebd2c217314d (commit)
      from  76ec9e2a123092c4bcedbab3f47779d1beebace1 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 5efbca2174eefd17f7b249a0662aeb30de4ec9c5
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Thu Oct 22 14:44:37 2020 -0400

    17010: Use !! instead of (? true : false)
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb
index 9baf82795..be463b022 100644
--- a/apps/workbench/app/controllers/container_requests_controller.rb
+++ b/apps/workbench/app/controllers/container_requests_controller.rb
@@ -178,7 +178,7 @@ class ContainerRequestsController < ApplicationController
 
     if params[:use_existing] || params[:use_existing].nil?
       # If nil, reuse workflow steps but not the workflow runner.
-      @object.use_existing = (params[:use_existing] ? true : false)
+      @object.use_existing = !!params[:use_existing]
 
       # Pass the correct argument to arvados-cwl-runner command.
       if command[0] == 'arvados-cwl-runner'

commit a71bf126349fba53bd0e90dc01dc17cfc656a7f4
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Wed Oct 21 22:27:14 2020 -0400

    17010: Don't double up --enable-reuse
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb
index 62ec3e150..9baf82795 100644
--- a/apps/workbench/app/controllers/container_requests_controller.rb
+++ b/apps/workbench/app/controllers/container_requests_controller.rb
@@ -182,14 +182,14 @@ class ContainerRequestsController < ApplicationController
 
       # Pass the correct argument to arvados-cwl-runner command.
       if command[0] == 'arvados-cwl-runner'
-        command -= ['--disable-reuse']
+        command -= ["--disable-reuse", "--enable-reuse"]
         command.insert(1, '--enable-reuse')
       end
     else
       @object.use_existing = false
       # Pass the correct argument to arvados-cwl-runner command.
       if command[0] == 'arvados-cwl-runner'
-        command -= ['--enable-reuse']
+        command -= ["--disable-reuse", "--enable-reuse"]
         command.insert(1, '--disable-reuse')
       end
     end

commit 9b77183d358c03dd74fced14bee920554e1cc94f
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Wed Oct 21 22:16:26 2020 -0400

    17010: Fix tests.  Tests check that --enable-reuse is set
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb
index c217eb977..62ec3e150 100644
--- a/apps/workbench/app/controllers/container_requests_controller.rb
+++ b/apps/workbench/app/controllers/container_requests_controller.rb
@@ -127,14 +127,12 @@ class ContainerRequestsController < ApplicationController
         @updates[:reuse_steps] = false
       end
       @updates[:command] ||= @object.command
+      @updates[:command] -= ["--disable-reuse", "--enable-reuse"]
       if @updates[:reuse_steps]
-        @updates[:command] = @updates[:command] - ["--disable-reuse"]
-        @updates[:command].insert(1, '--enable-reuse')
+        @updates[:command].insert(1, "--enable-reuse")
       else
-        @updates[:command] -= @updates[:command] - ["--enable-reuse"]
-        @updates[:command].insert(1, '--disable-reuse')
+        @updates[:command].insert(1, "--disable-reuse")
       end
-
       @updates.delete(:reuse_steps)
     end
 
@@ -167,26 +165,31 @@ class ContainerRequestsController < ApplicationController
         if arg.start_with? "--project-uuid="
           command[i] = "--project-uuid=#{@object.owner_uuid}"
         end
-        if arg == "--disable-reuse"
-          command[i] = "--enable-reuse"
-        end
       end
+      command -= ["--disable-reuse", "--enable-reuse"]
+      command.insert(1, '--enable-reuse')
+    end
+
+    if params[:use_existing] == "false"
+      params[:use_existing] = false
+    elsif params[:use_existing] == "true"
+      params[:use_existing] = true
     end
 
-    # By default the copied CR won't be reusing containers, unless use_existing=true
-    # param is passed.
-    if params[:use_existing]
-      @object.use_existing = true
+    if params[:use_existing] || params[:use_existing].nil?
+      # If nil, reuse workflow steps but not the workflow runner.
+      @object.use_existing = (params[:use_existing] ? true : false)
+
       # Pass the correct argument to arvados-cwl-runner command.
       if command[0] == 'arvados-cwl-runner'
-        command = src.command - ['--disable-reuse']
+        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']
+      if command[0] == 'arvados-cwl-runner'
+        command -= ['--enable-reuse']
         command.insert(1, '--disable-reuse')
       end
     end
diff --git a/apps/workbench/test/controllers/container_requests_controller_test.rb b/apps/workbench/test/controllers/container_requests_controller_test.rb
index c4a723930..73d357f3a 100644
--- a/apps/workbench/test/controllers/container_requests_controller_test.rb
+++ b/apps/workbench/test/controllers/container_requests_controller_test.rb
@@ -60,17 +60,19 @@ class ContainerRequestsControllerTest < ActionController::TestCase
   end
 
   [
-    ['completed', false, false],
-    ['completed', true, false],
+    ['completed',       false, false],
+    ['completed',        true, false],
+    ['completed',         nil, false],
     ['completed-older', false, true],
-    ['completed-older', true, true],
+    ['completed-older',  true, true],
+    ['completed-older',   nil, 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
+    test "container request #{uses_acr ? '' : 'not'} using arvados-cwl-runner copy #{reuse_enabled.nil? ? 'nil' : (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})
+      if !reuse_enabled.nil?
+        copy_params.merge!({use_existing: reuse_enabled})
       end
       post(:copy, params: copy_params, session: session_for(:active))
       assert_response 302
@@ -87,12 +89,11 @@ class ContainerRequestsControllerTest < ActionController::TestCase
       # 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_equal copied_cr['command'][0], 'arvados-cwl-runner'
+        if reuse_enabled.nil? || reuse_enabled
+          assert_includes copied_cr['command'], '--enable-reuse'
           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
diff --git a/apps/workbench/test/integration/work_units_test.rb b/apps/workbench/test/integration/work_units_test.rb
index 43740f192..4f2ebbc55 100644
--- a/apps/workbench/test/integration/work_units_test.rb
+++ b/apps/workbench/test/integration/work_units_test.rb
@@ -163,7 +163,9 @@ class WorkUnitsTest < ActionDispatch::IntegrationTest
       assert_text process_txt
       assert_selector 'a', text: template_name
 
-      assert_equal "Set value for ex_string_def", find('div.form-group > div > p.form-control-static > a', text: "hello-testing-123")[:"data-title"]
+      assert_equal "true", find('span[data-name="reuse_steps"]').text
+
+      assert_equal "Set value for ex_string_def", find('div.form-group > div.form-control-static > a', text: "hello-testing-123")[:"data-title"]
 
       page.assert_selector 'a.disabled,button.disabled', text: 'Run'
     end

commit 6fab25e336eb334525c8a506183281a350772d1f
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Wed Oct 21 17:19:05 2020 -0400

    17010: Fix tests
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb
index c42e93375..c217eb977 100644
--- a/apps/workbench/app/controllers/container_requests_controller.rb
+++ b/apps/workbench/app/controllers/container_requests_controller.rb
@@ -152,7 +152,7 @@ class ContainerRequestsController < ApplicationController
     @object = ContainerRequest.new
 
     # set owner_uuid to that of source, provided it is a project and writable by current user
-    if params[:work_unit][:owner_uuid]
+    if params[:work_unit].andand[:owner_uuid]
       @object.owner_uuid = src.owner_uuid = params[:work_unit][:owner_uuid]
     else
       current_project = Group.find(src.owner_uuid) rescue nil
@@ -161,7 +161,8 @@ class ContainerRequestsController < ApplicationController
       end
     end
 
-    if src.command[0] == 'arvados-cwl-runner'
+    command = src.command
+    if command[0] == 'arvados-cwl-runner'
       command.each_with_index do |arg, i|
         if arg.start_with? "--project-uuid="
           command[i] = "--project-uuid=#{@object.owner_uuid}"
@@ -174,11 +175,10 @@ class ContainerRequestsController < ApplicationController
 
     # 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'
+      if command[0] == 'arvados-cwl-runner'
         command = src.command - ['--disable-reuse']
         command.insert(1, '--enable-reuse')
       end
diff --git a/apps/workbench/test/controllers/container_requests_controller_test.rb b/apps/workbench/test/controllers/container_requests_controller_test.rb
index 140b59fa5..c4a723930 100644
--- a/apps/workbench/test/controllers/container_requests_controller_test.rb
+++ b/apps/workbench/test/controllers/container_requests_controller_test.rb
@@ -42,7 +42,7 @@ class ContainerRequestsControllerTest < ActionController::TestCase
     get :show, params: {id: uuid}, session: session_for(:active)
     assert_response :success
 
-    assert_includes @response.body, "action=\"/container_requests/#{uuid}/copy\""
+    assert_includes @response.body, "action_href=%2Fcontainer_requests%2F#{uuid}%2Fcopy"
   end
 
   test "cancel request for queued container" do

commit d4dd59e26aa0874d32145dcc0d91cfd36ac6948c
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Wed Oct 21 16:37:17 2020 -0400

    17010: Fix debug comment
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb b/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
index cc5d2dec5..7a9d68d98 100644
--- a/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
+++ b/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
@@ -21,7 +21,7 @@ SPDX-License-Identifier: AGPL-3.0 %>
                                                   }.to_json),
           { class: "btn btn-primary btn-sm", title: "Run #{@object.name}", remote: true }
           ) do %>
-      <i class="fa fa-fw fa-play"></i> Re-run...(2)
+      <i class="fa fa-fw fa-play"></i> Re-run...
     <% end %>
 
 <% end %>

commit 1ca30429c393325a091e3f7e30b36e204bbe2fca
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Wed Oct 21 16:13:50 2020 -0400

    17010: When running workflow from workbench, enable reuse by default
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/apps/workbench/app/controllers/work_units_controller.rb b/apps/workbench/app/controllers/work_units_controller.rb
index ba2f66abe..237cf2755 100644
--- a/apps/workbench/app/controllers/work_units_controller.rb
+++ b/apps/workbench/app/controllers/work_units_controller.rb
@@ -126,6 +126,7 @@ class WorkUnitsController < ApplicationController
       end
 
       attrs['command'] = ["arvados-cwl-runner",
+                          "--enable-reuse",
                           "--local",
                           "--api=containers",
                           "--project-uuid=#{params['work_unit']['owner_uuid']}",

commit d9b10653c157a46b77bb21ff5bffebd2c217314d
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Wed Oct 21 16:02:12 2020 -0400

    17010: Redesign "Re-run..." button to choose project to run in
    
    Also move workflow step reuse to the "Inputs" tab instead of a modal.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb
index 8ce068198..c42e93375 100644
--- a/apps/workbench/app/controllers/container_requests_controller.rb
+++ b/apps/workbench/app/controllers/container_requests_controller.rb
@@ -121,6 +121,23 @@ class ContainerRequestsController < ApplicationController
       end
     end
     params[:merge] = true
+
+    if !@updates[:reuse_steps].nil?
+      if @updates[:reuse_steps] == "false"
+        @updates[:reuse_steps] = false
+      end
+      @updates[:command] ||= @object.command
+      if @updates[:reuse_steps]
+        @updates[:command] = @updates[:command] - ["--disable-reuse"]
+        @updates[:command].insert(1, '--enable-reuse')
+      else
+        @updates[:command] -= @updates[:command] - ["--enable-reuse"]
+        @updates[:command].insert(1, '--disable-reuse')
+      end
+
+      @updates.delete(:reuse_steps)
+    end
+
     begin
       super
     rescue => e
@@ -134,6 +151,27 @@ class ContainerRequestsController < ApplicationController
 
     @object = ContainerRequest.new
 
+    # set owner_uuid to that of source, provided it is a project and writable by current user
+    if params[:work_unit][:owner_uuid]
+      @object.owner_uuid = src.owner_uuid = params[:work_unit][:owner_uuid]
+    else
+      current_project = Group.find(src.owner_uuid) rescue nil
+      if (current_project && current_project.writable_by.andand.include?(current_user.uuid))
+        @object.owner_uuid = src.owner_uuid
+      end
+    end
+
+    if src.command[0] == 'arvados-cwl-runner'
+      command.each_with_index do |arg, i|
+        if arg.start_with? "--project-uuid="
+          command[i] = "--project-uuid=#{@object.owner_uuid}"
+        end
+        if arg == "--disable-reuse"
+          command[i] = "--enable-reuse"
+        end
+      end
+    end
+
     # By default the copied CR won't be reusing containers, unless use_existing=true
     # param is passed.
     command = src.command
@@ -167,12 +205,6 @@ class ContainerRequestsController < ApplicationController
     @object.scheduling_parameters = src.scheduling_parameters
     @object.state = 'Uncommitted'
 
-    # 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
-    if (current_project && current_project.writable_by.andand.include?(current_user.uuid))
-      @object.owner_uuid = src.owner_uuid
-    end
-
     super
   end
 
diff --git a/apps/workbench/app/helpers/application_helper.rb b/apps/workbench/app/helpers/application_helper.rb
index 330d30976..786716eb3 100644
--- a/apps/workbench/app/helpers/application_helper.rb
+++ b/apps/workbench/app/helpers/application_helper.rb
@@ -247,11 +247,15 @@ module ApplicationHelper
     end
 
     input_type = 'text'
+    opt_selection = nil
     attrtype = object.class.attribute_info[attr.to_sym].andand[:type]
     if attrtype == 'text' or attr == 'description'
       input_type = 'textarea'
     elsif attrtype == 'datetime'
       input_type = 'date'
+    elsif attrtype == 'boolean'
+      input_type = 'select'
+      opt_selection = ([{value: "true", text: "true"}, {value: "false", text: "false"}]).to_json
     else
       input_type = 'text'
     end
@@ -279,6 +283,7 @@ module ApplicationHelper
       "data-emptytext" => '(none)',
       "data-placement" => "bottom",
       "data-type" => input_type,
+      "data-source" => opt_selection,
       "data-title" => "Edit #{attr.to_s.gsub '_', ' '}",
       "data-name" => htmloptions['selection_name'] || attr,
       "data-object-uuid" => object.uuid,
diff --git a/apps/workbench/app/models/container_request.rb b/apps/workbench/app/models/container_request.rb
index 48920c55e..be97a6cfb 100644
--- a/apps/workbench/app/models/container_request.rb
+++ b/apps/workbench/app/models/container_request.rb
@@ -22,4 +22,24 @@ class ContainerRequest < ArvadosBase
   def work_unit(label=nil, child_objects=nil)
     ContainerWorkUnit.new(self, label, self.uuid, child_objects=child_objects)
   end
+
+  def editable_attributes
+    super + ["reuse_steps"]
+  end
+
+  def reuse_steps
+    command.each do |arg|
+      if arg == "--enable-reuse"
+        return true
+      end
+    end
+    false
+  end
+
+  def self.attribute_info
+    self.columns
+    @attribute_info[:reuse_steps] = {:type => "boolean"}
+    @attribute_info
+  end
+
 end
diff --git a/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb b/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
index b698c938a..cc5d2dec5 100644
--- a/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
+++ b/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
@@ -9,40 +9,19 @@ SPDX-License-Identifier: AGPL-3.0 %>
   }
 </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">×</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>
+    <%= link_to(choose_projects_path(id: "run-workflow-button",
+                                     title: 'Choose project',
+                                     editable: true,
+                                     action_name: 'Choose',
+                                     action_href: copy_container_request_path,
+                                     action_method: 'post',
+                                     action_data: {'selection_param' => 'work_unit[owner_uuid]',
+                                                   'work_unit[template_uuid]' => @object.uuid,
+                                                   'success' => 'redirect-to-created-object'
+                                                  }.to_json),
+          { class: "btn btn-primary btn-sm", title: "Run #{@object.name}", remote: true }
+          ) do %>
+      <i class="fa fa-fw fa-play"></i> Re-run...(2)
     <% end %>
-  </div>
-</div>
 
 <% end %>
diff --git a/apps/workbench/app/views/container_requests/_show_inputs.html.erb b/apps/workbench/app/views/container_requests/_show_inputs.html.erb
index fd8e36383..07bf7c4d7 100644
--- a/apps/workbench/app/views/container_requests/_show_inputs.html.erb
+++ b/apps/workbench/app/views/container_requests/_show_inputs.html.erb
@@ -17,23 +17,23 @@ n_inputs = if @object.mounts[:"/var/lib/cwl/workflow.json"] && @object.mounts[:"
     <% if workflow %>
       <% inputs = get_cwl_inputs(workflow) %>
       <% inputs.each do |input| %>
-        <label for="#input-<%= cwl_shortname(input[:id]) %>">
-          <%= input[:label] || cwl_shortname(input[:id]) %>
-        </label>
-        <div>
-          <p class="form-control-static">
-            <%= render_cwl_input @object, input, [:mounts, :"/var/lib/cwl/cwl.input.json", :content] %>
+        <div class="form-control-static">
+          <label for="#input-<%= cwl_shortname(input[:id]) %>">
+            <%= input[:label] || cwl_shortname(input[:id]) %>
+          </label>
+          <%= render_cwl_input @object, input, [:mounts, :"/var/lib/cwl/cwl.input.json", :content] %>
+          <p class="help-block">
+            <%= input[:doc] %>
           </p>
         </div>
-        <p class="help-block">
-          <%= input[:doc] %>
-        </p>
       <% end %>
     <% end %>
   </div>
 </form>
 <% end %>
 
+<p style="margin-bottom: 2em"><b style="margin-right: 3em">Reuse past workflow steps if available?</b>  <%= render_editable_attribute(@object, :reuse_steps) %></p>
+
 <% if n_inputs == 0 %>
   <p><i>This workflow does not need any further inputs specified.  Click the "Run" button at the bottom of the page to start the workflow.</i></p>
 <% else %>

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list