Arvados

"Re-run latest version" here runs the job again, with a newer git commit (if compatible with the given script_version). I'm guessing what you wanted/expected was to run a new pipeline using the latest version of the template?

ah yes, I thought it would work like the run with latest button.

Proposal: Reformat the "re-run" choices to provide space to explain the options in sufficient detail.

"Repeat this job..." button opens a dialog.

• "Same version": Use exactly the same script version (commit hash) as the current job. This is useful if you're retrying a job that failed due to system problems, or if you're verifying that you can repeat the job and get the same output.
• "Current version": Use the current commit indicated by "foo" in the "bar" repository. This is useful for testing a new version on the same branch. (In this description, "foo" is the current job's supplied_script_version, and "bar" is its repository.)
• (Future option -- possibly not for this story if it's trivial, but conceptually it would go here) "Another version": Editable repository and version fields, pre-populated with the current job's repository and supplied_script_version.

• The inputs and parameters will be exactly the same as in the current job. That means, for example, the new job will not reflect any changes made to the pipeline that initiated this job.
• If this job is part of a pipeline, that pipeline won't know about the new job you're running. If you're hoping to update your pipeline results, you're in the wrong place: you should re-run the pipeline instead.

Branch 4878-rerun-job ready for review.

• I copied over 7 attributes from the current job instance (script, script_version, repository, supplied_script_version, nondeterministic, script_parameters, runtime_constraints) to the newly created job instance. This is what the "Re-run same version" and the previous implementation are doing. Please let me know if I am missing any other attributes. Also, please confirm the "repository" value is correct in the newly created job instance.

• The tests do not actually succeed in "re-running" the jobs (fixture based jobs) and I see fiddlesticks with error message "Script version master does not resolve to a commit" and hence I used assertions to ensure the UI to re-run works as expected. If it is possible to re-run these jobs, please let me know.

Thanks.

Radhika Chippada wrote:

Branch 4878-rerun-job ready for review.

Reviewing 2926892

The story¶

I understood Tom's comment to suggest a single button, "Repeat this job…", that offers the modal dialog that you've implemented. The branch currently leaves in the "Re-run same version" button, which is redundant with clicking "Re-run with options" and choosing "Use same script version."

I see that you currently make a distinction about whether or not it's possible to "Re-run with latest," based on whether or not supplied_script_version is a symbolic reference. That makes sense to me. However, I would've expected things to consolidate so that there's still a single button. If it's not possible to find a latest version, that radio option could be disabled with explanatory text.

But maybe I missed some discussion about the interface considerations on IRC. Were there changes planned?

Controller code¶

Because this branch is implementing a pure interface change, I don't believe it should be necessary to add code to the controller at all. The HTML for the modal can be added directly to the show template, inside the div you've added. And then the modal form can create a new job by POST to /jobs, using the existing route and hidden fields like the previous code. The radio buttons in the modal can set the script_version directly from supplied_script_version or script_version, based on the user's selection, rather than having a separate use_script parameter. Implementing the story along these lines would help cut down the amount of code we have to maintain.

Modal interface¶

One small thing, the labels in the label don't have the right "label_for" strings to be attached to the radio buttons. Normally clicking a label should toggle the associated form element. That's not happening inside the modal.

• I copied over 7 attributes from the current job instance (script, script_version, repository, supplied_script_version, nondeterministic, script_parameters, runtime_constraints) to the newly created job instance. This is what the "Re-run same version" and the previous implementation are doing. Please let me know if I am missing any other attributes. Also, please confirm the "repository" value is correct in the newly created job instance.

This all looks correct to me.

• The tests do not actually succeed in "re-running" the jobs (fixture based jobs) and I see fiddlesticks with error message "Script version master does not resolve to a commit" and hence I used assertions to ensure the UI to re-run works as expected. If it is possible to re-run these jobs, please let me know.

I don't think we have the infrastructure to make that possible from Workbench tests. The API server tests have a Git repository helper to fill out a small testing repository based on demand. However, I don't believe it's possible to create/invoke it from the Workbench tests. I assume fixing that is more than you want to take on in a half-point story. :)

Thanks.

• I removed the “re-run using same version” button. In the re-run dialog, if a job has no newer version, I am hiding the “re-run using latest” button. In this case, the popup shows one radio button which is already selected, which is like an oxymoron. I experimented with various display options.
  • In this case, instead of the popup, I can display just the button. However, in this case, the good “if this job is part of a pipeline” explanation is not available to the user. And, I think this is useful. I could present this as a tooltip in this case.
  • Alternatively, I can show no radio buttons in the popup. So, the popup will just have the “if this is part of a pipeline” explanation and the “Run now” and Cancel buttons.
  • Or, we can stick with how it is now, which is to show the popup with a single ratio button option.

• As I described in IRC, I am passing supplied_script_version for the new job post request to avoid the situation where “re-run same” ends up setting the script_version as the supplied script version and results in losing it.

• Controller code: I spent several hours (before implementing it this way) as well as yesterday to do the modal implementation in the show template itself but I could not get it working. Hence, I am leaving it as is. I do think this is easier to read and understand. It will also be probably easier if we want to expand it further in the future to add “future options”. But, I have no strong preference either way, just that I could not get it working :).

Radhika Chippada wrote:

• I removed the “re-run using same version” button. In the re-run dialog, if a job has no newer version, I am hiding the “re-run using latest” button. In this case, the popup shows one radio button which is already selected, which is like an oxymoron. I experimented with various display options.
  • In this case, instead of the popup, I can display just the button. However, in this case, the good “if this job is part of a pipeline” explanation is not available to the user. And, I think this is useful. I could present this as a tooltip in this case.

I agree with both these points: a popup with one radio button is kind of weird, but we always want users to see the information about how rerunning jobs doesn't affect pipelines.

There's enough information there that I'm worried it would be kind of cramped to fit in a tooltip. What if we always showed both radio buttons, but disabled the "use latest version" radio button when the source job's supplied_script_version is a commit hash?

• Alternatively, I can show no radio buttons in the popup. So, the popup will just have the “if this is part of a pipeline” explanation and the “Run now” and Cancel buttons.

That sounds reasonable to me too, if you prefer it.

• Controller code: I spent several hours (before implementing it this way) as well as yesterday to do the modal implementation in the show template itself but I could not get it working. Hence, I am leaving it as is. I do think this is easier to read and understand.

I feel strongly that it's important to avoid repeating code as much as possible. Having two Workbench controller methods to create jobs means there are two places where that process can be buggy, and two places where bugs would need to be fixed. I feel it's much preferable for long-term software maintainability to reuse the existing method.

It will also be probably easier if we want to expand it further in the future to add “future options”.

I'm not sure I see anything that would prevent us from adding those sorts of options to the existing job creation method.

But, I have no strong preference either way, just that I could not get it working :).

Mdoals are mostly hooked up with a bunch of data-* attributes on the relevant elements. This does have the downside that you have to be careful that attribute names and values all line up exactly, or else the JavaScript won't function correctly. But the Bootstrap modal documentation is pretty thorough and walks through several different use cases. Ours seems to be most like the "Live demo" under "Examples." Do you think that might be helpful?

If you're still having trouble, it might help to have the JavaScript console open while you're testing it. Bootstrap might write out a warning if it tries to open a modal that doesn't exist, or other issues like that which could help track down trouble.

Thanks.

Brett Smith wrote:

Because this branch is implementing a pure interface change, I don't believe it should be necessary to add code to the controller at all. The HTML for the modal can be added directly to the show template, inside the div you've added. And then the modal form can create a new job by POST to /jobs, using the existing route and hidden fields like the previous code. The radio buttons in the modal can set the script_version directly from supplied_script_version or script_version, based on the user's selection, rather than having a separate use_script parameter. Implementing the story along these lines would help cut down the amount of code we have to maintain.

I have prepared a branch implemented this way. It's up for review at 4878-rerun-job-bcs-wip. All the interface is the same, it just flattens out the supporting code.