Generic GIE Launcher, GIE Image Chooser, multiple datasets#1403
Generic GIE Launcher, GIE Image Chooser, multiple datasets#1403jmchilton merged 51 commits intogalaxyproject:devfrom
Conversation
|
This is good functionality but it I don't think it should be an interactive environment, it should just be a prominent Galaxy component somewhere? You've developed an awesome hammer inside of Galaxy and everything looks like a nail. @bgruening were you involved in the discussion of where to put an interactive environments launcher? Or does anyone remember? Did that conversation end with a conclusion? Did we decided somewhere in the history panel or as a top-level tab? I'm happy to keep it this way for now, but if it is easy enough to stick this mako somewhere else - I think we should attempt to. |
|
Might have missed it, but Can you do that so the JavaScript can't launch arbitrary docker images. |
Technically a viz plugin, but yes, viz plugins are just so much easier to build than galaxy infra (and so much less bike-shedding involved) so I developed it as such. A proper launcher could be a more tightly coupled component. It wouldn't be too hard.
We three discussed having some sort of history/global level launcher, but that discussion never went anywhere iirc. Just a "this is a future concern," unfortunately this is a "now" concern so let's open that up for discussion. I really like having these things in the viz menu where people expect them. Moving them somewhere else breaks the user's expectations. A top level tab would be quite prominent, but I don't think we ever developed these with such grand expectations for galaxy adoption. If that's on the table, great. Let's do that.
It should be trivial to stick somewhere else. Doing it right will be a bit of engineering working to wire up APIs for listing GIEs + images. That's the only feature that really needs to be implemented for it to be embedded the right way. Everything else can be done with existing APIs. |
|
@erasche Awesome, thanks! |
|
Awesome @erasche! I feel we need to improve the usability a little bit. More choices always means for freedom but also more confusion :) We should include a help section or a description for the images that can guide users in a certain way. @jmchilton yes I did participate. We agreed on a two fold solution as far as I remember. For universal IE's something in the top header would be nice. Maybe the visualisation menu but renaming it. Thanks Eric & John! 🍻 |
Yes, I considered doing that but didn't feel like writing description datasets for each and every GIE that would be automatically loaded. If you think it is worth doing too, then I suppose we have to do that :P that's at least an easy fix. I'll have that one out shotrly
Ah, better memory than mine. I'm not sure how one goes about adding things to the top header, so I may need some pointers there. |
Two fixes to gie generic stuff..
Fixup sample comments.
| elif os.path.exists(os.path.join(self.attr.our_config_dir, 'allowed_images.yml.sample')): | ||
| fn = os.path.join(self.attr.our_config_dir, 'allowed_images.yml.sample') | ||
| else: | ||
| raise Exception("Could not find allowed_images.yml file for " + self.attr.viz_id) |
There was a problem hiding this comment.
It seems to me like this requires an allowed_images.yml file (or sample) - and phinch and iobio don't have one. Can we fallback to just the ini for these simpler IEs that function more like a single dataset viz (where the history panel button make a lot more sense).
There was a problem hiding this comment.
Yes, absolutely. That was the original idea, I'm not, for the life of me, understanding why I raised an exception here. I definitely shouldn't be.
| target : '_frame' | ||
| }] | ||
| },{ | ||
| title : 'Galaxy Interactive Environments (GIEs)', |
There was a problem hiding this comment.
Do we really need to say here Galaxy? Or is it ok to call them simply Interactive Environments
There was a problem hiding this comment.
We'd always called them IEs internally but there seemed to be pressure to name it GIEs. Was trying to be consistent
|
@bgruening when you get a chance...any more comments on this? |
|
Comments from IRL discussion with @jmchilton:
|
hexylena
changed the title
|
Ok, datasets are no longer mounted as volumes. The only remaining issue is that for some reason I get the error that the jupyter IE is invalid mako, while rstudio is OK. They look identical, so maybe I'm missing something? I would really appreciate a second set of eyes if anyone has time. |
|
Containers will need to be updated to take advantage of this new feature. Otherwise they will only load the first dataset (or first notebook/rdata file, if one of those is available) |
|
@erasche @jmchilton can you give a few reason behind the decision to not mount volumes? I kind of regret to have this option removed from Jupyter. It is now not possible to work with large files. Reading a BAM file or a strange proteomics XML blob is not possible if you need to copy GBs over the wire into a Docker container. Just curious. I will try to get this running over the weekend. |
|
@bgruening I was under the impression you don't use it? And main doesn't as well afaik? I can add the option back in if needed. There was more reasoning, will try and reply later |
|
Main doesn't mount the Galaxy datasets directory (directories?) on the Docker host and I think this is for the best right now. We should probably proceed with the assumption that the Docker host can be compromised fairly easily from an interactive environment like Jupyter - this would expose all the data to the Docker container not just the requested files. An option or flag to re-enable direct mounting when it is available sounds good to me - but I don't think it has to be part of this PR and I think it should probably be off by default? |
A fix to my previous fix.
|
@jmchilton willing to add as part of this PR. |
Conflicts: static/maps/layout/menu.js.map static/scripts/bundled/libs.bundled.js static/scripts/bundled/libs.bundled.js.map static/scripts/layout/menu.js
5 participants
Long have we had to suffer under the tyranny of only being able to configure a single image per GIE. This unnecessary restriction must go! 😸
$GALAXY_ROOT/config/plugins/interactive_environments/$GALAXY_ROOT/config/plugins/interactive_environments/<GIE>/config/allowed_images.txt.sample. Admins may specify as many images as they like, and they will all show up under this select box./import/{dataset_name}.{dataset_ext}Fröhliche Weihnachten und alles Gute zum Geburtstag @bgruening.
Tagged 16.04 but feel free to try and get this in in 16.01, is 15.10 even out yet?
cc @jmchilton @bgruening