Story #11185

[Workbench] Add ability to disable job reuse when running a workflow

Added by Tom Morris about 1 month ago. Updated about 14 hours ago.

Status:In ProgressStart date:03/01/2017
Priority:NormalDue date:
Assignee:Lucas Di Pentima% Done:

0%

Category:-
Target version:2017-05-10 sprint
Story points1.0Remaining (hours)0.00 hour
Velocity based estimate-

Description

  • Either "re-run container request" button should present a dialog box asking the user if they want job reuse or not, or there should be 2 separate buttons that are clearly labeled
  • Must recognize that a container request for "arvados-cwl-runner" and add "--disable-reuse" to the command field.

Subtasks

Task #11471: Review 11185-wb-disable-reuseNewRadhika Chippada

History

#1 Updated by Tom Morris about 1 month ago

  • Target version changed from 2017-03-15 sprint to Arvados Future Sprints

#2 Updated by Tom Morris about 1 month ago

  • Target version changed from Arvados Future Sprints to 2017-04-12 sprint
  • Story points set to 1.0

#3 Updated by Peter Amstutz about 1 month ago

  • Description updated (diff)
  • Target version changed from 2017-04-12 sprint to Arvados Future Sprints

#4 Updated by Tom Morris 17 days ago

  • Target version changed from Arvados Future Sprints to 2017-04-26 sprint

#5 Updated by Lucas Di Pentima 17 days ago

  • Assignee set to Lucas Di Pentima

#6 Updated by Lucas Di Pentima 4 days ago

  • Status changed from New to In Progress

#7 Updated by Lucas Di Pentima 3 days ago

Branch 11185-wb-disable-reuse - e7b46691f
Test run: https://ci.curoverse.com/job/developer-run-tests/252/

Added "Re-run without reuse" button on the CR show page, passing params[:no_reuse] to the copy controller method.
When this happens, it passes '--disable-reuse' to the arvados-cwl-runner call.

Pending: Check what other cases should be covered.

#8 Updated by Lucas Di Pentima 3 days ago

Update: fed8ee006
Test run: https://ci.curoverse.com/job/developer-run-tests/253/

Updating cr.use_existing to false was missing when copying CR for a re-run.
Added related test.

#9 Updated by Radhika Chippada 3 days ago

  • Can you rename “Re-run” as “Re-run with reuse”. You might also want to switch the two buttons. First “Re-run with reuse” and then “Re-run without reuse”
  • Please update the tooltip on “Re-run with reuse” to say that the container will be reused
  • Initializing command as follows might be better if there is a scenario such as params[:no_reuse] but src.command ! = ‘arvados-cwl-runner’ ?
+    command = src.command
     if params[:no_reuse]
       @object.use_existing = false
       # If "no reuse" requested, pass the correct argument to arvados-cwl-runner command.
@@ -71,8 +72,6 @@ class ContainerRequestsController < ApplicationController
         command = src.command - ['--enable-reuse']
         command.insert(1, '--disable-reuse')
       end
-    else
-      command = src.command
     end

#10 Updated by Radhika Chippada 3 days ago

Please merge the two Re-run buttons into one Re-run button with a popup (As we discussed in the Review meeting)

The popup presents the user with one option (a checkbox) to enable / disable reuse. Default is "checked" which means "reuse". Ok and Cancel buttons to close the popup. (You can alternatively use two radio buttons with the reuse being default). Please see pipeline instance "re-run options" for a similar usage.

#11 Updated by Lucas Di Pentima 3 days ago

  • Target version changed from 2017-04-26 sprint to 2017-05-10 sprint

#12 Updated by Lucas Di Pentima 1 day ago

Updates at: 65da23323
Test run: https://ci.curoverse.com/job/developer-run-tests/260/

Discovered that the previous default behavior of the Re-run button was to copy the CR with the reuse feature disabled, so this story is about allowing the user to enable reuse.
Replaced the additional Re-run button with a modal dialog that has a checkbox to enable the feature.
Also found a bug where the CR.use_existing attr was set to false by default but the command wasn't updated when used arvados-cwl-runner, so any subsequent CR created would be re-using jobs.

#13 Updated by Radhika Chippada 1 day ago

  • link_to raw('<i class="fa fa-fw fa-play"></i> Re-run') => Please use the link title 'Re-run ...' to indicate that this presents a popup
  • By default the copied CR won't be reusing jobs ... => since the reused objects are containers, can you say so instead of "jobs"?
  • Not a big deal since we only have one checkbox currently, but "reset_form" is not working in the popup. Click on Re-use and click on Cancel and click on Re-use again and see that the checkbox is still checked.
  • In test "container request copy with reuse enabled", can we also check that arvados-cwl-runner flag is set correctly? Also, can we check this flag in test "container request copy"?

Thanks.

#14 Updated by Lucas Di Pentima about 14 hours ago

Addressed all previous comments at d694a717a
Test run: https://ci.curoverse.com/job/developer-run-tests/263/

Also available in: Atom PDF