Story #10645

[Workbench] Better presentation of container request "mounts"

Added by Peter Amstutz over 4 years ago. Updated about 4 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
-
Target version:
Start date:
05/08/2017
Due date:
% Done:

100%

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

Description

Remove command, cwd, and mounts.

Replace mounts field with rendered version of important fields, including: runtime_constraints, others TBD (ask Bryan)

If present, the mount "/var/lib/cwl/cwl.input.json" should be rendered in a separate box or table as inputs to the workflow. Input files or collections should have hyperlinks to view the collection or file.

If "/var/lib/cwl/workflow" is a collection mount, there should be a hyperlink to the collection.

The rest of the contents of "mounts" should be hidden from the main work unit display.

If template_uuid is present, link it to the template that was used to create it.

cr-mounts-display.png (66.7 KB) cr-mounts-display.png Radhika Chippada, 05/01/2017 06:25 PM
cr-mounts-display-2.png (47.5 KB) cr-mounts-display-2.png Radhika Chippada, 05/02/2017 01:42 AM

Subtasks

Task #11566: Review 10645-cr-mounts-displayResolvedPeter Amstutz

Associated revisions

Revision 8ee2f839
Added by Radhika Chippada about 4 years ago

refs #10645
Merge branch '10645-cr-mounts-display'

Revision 7f1e7f79
Added by Radhika Chippada about 4 years ago

closes #10645
Merge branch '10645-cr-mounts-display'

History

#1 Updated by Peter Amstutz over 4 years ago

  • Description updated (diff)

#2 Updated by Peter Amstutz over 4 years ago

  • Story points set to 1.0

#3 Updated by Tom Morris about 4 years ago

  • Target version set to Arvados Future Sprints

#4 Updated by Tom Morris about 4 years ago

  • Description updated (diff)
  • Assigned To set to Tom Morris

#5 Updated by Tom Morris about 4 years ago

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

#6 Updated by Tom Morris about 4 years ago

  • Assigned To deleted (Tom Morris)

#7 Updated by Tom Morris about 4 years ago

  • Release deleted (11)

#8 Updated by Tom Morris about 4 years ago

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

#9 Updated by Radhika Chippada about 4 years ago

  • Assigned To set to Radhika Chippada

#10 Updated by Radhika Chippada about 4 years ago

  • Status changed from New to In Progress

#11 Updated by Radhika Chippada about 4 years ago

9:52:04 AM radhika: ok. remove cwd, environment and output_path?
9:52:26 AM tetron: yes

#12 Updated by Radhika Chippada about 4 years ago

Added a mockup of the container request page with the additional items. Please see cr-mounts-display.png

#13 Updated by Peter Amstutz about 4 years ago

Radhika Chippada wrote:

Added a mockup of the container request page with the additional items. Please see cr-mounts-display.png

  • Make "command" full width and displayed below the two-column section.
  • Make "inputs" full width and displayed below "command". It should render the contents of /var/lib/cwl/cwl.input.json as JSON or YAML with clickable links.
  • The "mounts" box should have a maximum height of at least 5 lines.
  • The "inputs" box should have a maximum height of at least 15 lines.

#14 Updated by Radhika Chippada about 4 years ago

#15 Updated by Radhika Chippada about 4 years ago

Branch 10645-cr-mounts-display @ c3aa32a

#16 Updated by Peter Amstutz about 4 years ago

  • Please remove the borders around the values of "runtime_constraints", "command" and "input_mounts". Instead, can we make the labels vertically top-aligned instead of center-aligned? That will make it easier to tell where multi-line values start and end.
  • When /var/lib/cwl/cwl.input.json is present the JSON should be rendered explicitly (with a clickable collection link):
"input": {
  "class": "File",
  "location": "keep:cba47aefe5eb3a014a26ec00316b30c1+57/whale.txt",
  "basename": "whale.txt" 
  },
  "reverse_sort": true
}

We could generalize this by going through "mounts" and rendering each one which is either {"kind": "collection"} or {"kind": "json"}. The slightly tricky part is that "/var/lib/cwl/workflow.json" is usually going to be large, so it should be limited/hidden by default, whereas "/var/lib/cwl/cwl.input.json" is essential information and shouldn't be hidden by default.

#17 Updated by Peter Amstutz about 4 years ago

While we're at it, can we look at "requesting_container_uuid" and provide a link to the container and/or a link to the container request for that container?

#18 Updated by Radhika Chippada about 4 years ago

Please remove the borders around the values of "runtime_constraints", "command" and "input_mounts". Instead, can we make the labels vertically top-aligned instead of center-aligned? That will make it easier to tell where multi-line values start and end.

Done and valign=top works for this

When /var/lib/cwl/cwl.input.json is present the JSON should be rendered explicitly (with a clickable collection link)

Improved the display to display the whole cwl.input.json with collection links rendered clickable when applicable

We could generalize this by going through "mounts" and rendering each one which is either {"kind": "collection"} or {"kind": "json"} ...

For the time being I changed the title of the "/var/lib/cwl/cwl.input.json" field to clarify that it is cwl.input.json, so that there is no confusion when this is not present, but mounts does have inputs that are displayed in the graph. We can expand it to walk through mounts and display desired parts of mounts in the future if we decide to go this path, but haven't done it at this time. How common is it to indicate inputs in mounts using a format other than /var/lib/cwl/cwl.input.json?

While we're at it, can we look at "requesting_container_uuid" and provide a link to the container and/or a link to the container request for that container?

Done

#19 Updated by Peter Amstutz about 4 years ago

Using qr1hi-dz642-uypqs2exiy7so7l as my test case.

cwl.input.json display feels cramped. Suggest making it taller, like 40em.

If I make the window narrow so that it switches from two-column display to one-column display, the "command" and "cwl.input.json" fields get into trouble. They run off the right hand edge of the screen.

Suggest moving "priority" from the top of the right hand column to the bottom of the left hand column.

The collection hyperlinks should include the "keep:" prefix and quotes around the link, so that the input display can be copy and pasted as valid CWL input JSON.

There isn't good visual separation between the elements. Consider apply the bootstrap table style: https://getbootstrap.com/css/#tables

I played around with class="table table-condensed" and it made it much easier to distinguish the various fields we are trying to present.

#20 Updated by Radhika Chippada about 4 years ago

@ aa6a63f

Addressed all the comments from the note 19.

#21 Updated by Peter Amstutz about 4 years ago

I pushed a couple of layout tweaks. Now @ 75a76c083c8f257c33ad31582aac7a86bc866bb5

Trying to get this finished up, but I have two more requests:

  • When displaying cwl.input.json, can the PDH part and the file part (where it appears) be separate links? Sometimes I want to go to the collection, sometimes I want to download the file. Currently if it is a file reference, the link is only to download the file.
  • Would it be possible to make PDHs clickable in the display of the command line? Maybe the code for displaying the command line and the displaying cwl.input.json can be refactored into one routine which adds the links.

#22 Updated by Peter Amstutz about 4 years ago

Example HTML linking PDH and file separately:

"location": "keep:<a href='/collections/f0469339ca1d9a51e18f4b011714bfaf+66'>f0469339ca1d9a51e18f4b011714bfaf+66</a>/<a href='/collections/f0469339ca1d9a51e18f4b011714bfaf+66/assignMultiVars.pl'>assignMultiVars.pl</a>" 

And something similar for the display of the command:

command: ["/keep/<a href='/collections/f0469339ca1d9a51e18f4b011714bfaf+66'>f0469339ca1d9a51e18f4b011714bfaf+66</a>/<a href='/collections/f0469339ca1d9a51e18f4b011714bfaf+66/assignMultiVars.pl'>assignMultiVars.pl</a>"]

#23 Updated by Radhika Chippada about 4 years ago

@ 2c60e15078452202ff58eac99d44920448fb7b9a

Addressed comments from note 21.

To test: container_requests/9tee4-xvhdp-2jb8m4atfsq1wdj has collections in cwl.input.json and expanding first child "workflow.json#main" has command with collection links.

#24 Updated by Peter Amstutz about 4 years ago

Looks great @ 2c60e15078452202ff58eac99d44920448fb7b9a Please merge.

#25 Updated by Peter Amstutz about 4 years ago

The container request panel seems to be missing a "Log" link.

#26 Updated by Ward Vandewege about 4 years ago

There is a bug in the link to the files inside collections: the link is url-encoded, which means the download doesn't actually work.

For example, on https://workbench.9tee4.arvadosapi.com/container_requests/9tee4-xvhdp-7fth5jo49uen9w1, the link to 'args.py' is currently

https://workbench.9tee4.arvadosapi.com/collections/de738550734533c5027997c87dc5488e+53%2Fargs.py

but it should be

https://workbench.9tee4.arvadosapi.com/collections/de738550734533c5027997c87dc5488e+53/args.py

#27 Updated by Radhika Chippada about 4 years ago

  • Target version changed from 2017-05-10 sprint to 2017-05-24 sprint
  • Story points changed from 1.0 to 0.0

#28 Updated by Radhika Chippada about 4 years ago

Branch 10645-cr-mounts-display @ 97d759d

The container request panel seems to be missing a "Log" link.

We use "keys << :log_collection if @object.uuid != current_obj.uuid" to determine whether or not to show the log link. That is, for the main object, the log is shown in the Log tab. If you expand a child, you will see that the child CRs show the log link. Since this was the earlier direction from Tom, I am leaving it alone. Let me know if we want to change this now and we can discuss with Tom.

There is a bug in the link to the files inside collections: the link is url-encoded, which means the download doesn't actually work.

I changed the collection link generation to use the same logic as collections/_show_files, with disposition based on another feedback I heard from Tom during the review and this is working as expected now; I can remove the disposition if you do not want it. (BTW, the "link_to_arvados_object_if_readable" is generating link with encoding and may be a Rails issue that might come back in other use cases)

#29 Updated by Peter Amstutz about 4 years ago

10645-cr-mounts-display LGTM @ 97d759d73e24017c9cab74325d866960389c11a5

#30 Updated by Radhika Chippada about 4 years ago

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

Applied in changeset arvados|commit:7f1e7f793bd1b5efa73df51b3070c5acb8fcdc82.

Also available in: Atom PDF