Project

General

Profile

Actions

Idea #11185

closed

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

Added by Tom Morris about 7 years ago. Updated almost 7 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
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 1 (0 open1 closed)

Task #11471: Review 11185-wb-disable-reuseResolvedRadhika Chippada03/01/2017Actions
Actions #1

Updated by Tom Morris about 7 years ago

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

Updated by Tom Morris about 7 years ago

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

Updated by Peter Amstutz about 7 years ago

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

Updated by Tom Morris almost 7 years ago

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

Updated by Lucas Di Pentima almost 7 years ago

  • Assigned To set to Lucas Di Pentima
Actions #6

Updated by Lucas Di Pentima almost 7 years ago

  • Status changed from New to In Progress
Actions #7

Updated by Lucas Di Pentima almost 7 years 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.

Actions #8

Updated by Lucas Di Pentima almost 7 years 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.

Actions #9

Updated by Radhika Chippada almost 7 years 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
Actions #10

Updated by Radhika Chippada almost 7 years 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.

Actions #11

Updated by Lucas Di Pentima almost 7 years ago

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

Updated by Lucas Di Pentima almost 7 years 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.

Actions #13

Updated by Radhika Chippada almost 7 years 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.

Actions #14

Updated by Lucas Di Pentima almost 7 years ago

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

Actions #15

Updated by Radhika Chippada almost 7 years 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
Actions #16

Updated by Lucas Di Pentima almost 7 years 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.
Actions #17

Updated by Lucas Di Pentima almost 7 years ago

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

Applied in changeset arvados|commit:fca805a18c671ccbb03cef640c15172d1f02ffe3.

Actions

Also available in: Atom PDF