Story #11185

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

Added by Tom Morris 10 months ago. Updated 7 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
03/01/2017
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
1.0

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-reuseResolvedRadhika Chippada

Associated revisions

Revision fca805a1
Added by Lucas Di Pentima 7 months ago

Merge branch '11185-wb-disable-reuse'
Closes #11185

History

#1 Updated by Tom Morris 10 months ago

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

#2 Updated by Tom Morris 9 months ago

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

#3 Updated by Peter Amstutz 9 months ago

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

#4 Updated by Tom Morris 8 months ago

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

#5 Updated by Lucas Di Pentima 8 months ago

  • Assigned To set to Lucas Di Pentima

#6 Updated by Lucas Di Pentima 8 months ago

  • Status changed from New to In Progress

#7 Updated by Lucas Di Pentima 8 months 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 8 months 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 8 months 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 8 months 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 8 months ago

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

#12 Updated by Lucas Di Pentima 8 months 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 8 months 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 8 months ago

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

#15 Updated by Radhika Chippada 7 months ago

  • It appears that you might be able to change the cmd for the completed-old CR fixture rather than adding another "completed-acr" fixture
  • You might be able to simplify the name of the test “container request #{uses_acr} …” by removing the user_acr in the name
  • In _extra_tab_line_buttons html, you might want to move the reset_form function to the bottom of the file outside the <% ... %> code to improve readability

#16 Updated by Lucas Di Pentima 7 months ago

Updates at c6452ff3c
Test run: https://ci.curoverse.com/job/developer-run-tests/265/

  • Kept JS code inside the template's if statement so it's rendered only when necessary. As requested, changed the function's name to avoid conflicts.
  • Removed the unnecessary completed-acr CR fixture and modified the command of completed-older to be useful for the added tests.

#17 Updated by Lucas Di Pentima 7 months ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:fca805a18c671ccbb03cef640c15172d1f02ffe3.

Also available in: Atom PDF