galaxyproject / ansible-galaxy-tools

An Ansible role for automated installation of tools from a Tool Shed into Galaxy.
MIT License
14 stars 34 forks source link

Fix for installing a tool into a new tool panel section #34

Closed gregvonkuster closed 8 years ago

gregvonkuster commented 8 years ago

The bioblend API has the following method signature for ~/bioblend/galaxy/toolsehd:

def install_repository_revision(self,
    tool_shed_url,
    name,
    owner,
    changeset_revision,
    install_tool_dependencies=False,
    install_repository_dependencies=False,
    tool_panel_section_id=None,
    new_tool_panel_section_label=None):

This PR replaces the use of the incorrect "tool_panel_section_label" to be "new_tool_panel_section_label", which is supported in bioblend.

This will fix tool installs that use the bioblend API for installing tools into new tool panel sections (e.g., docker containers for Galaxy flavors" ping @bgruening

bgruening commented 8 years ago

I'm really wondering why this is in the docs: https://github.com/bgruening/galaxy-metagenomics/blob/master/metagenomics.yaml#L17 It seems it was never used and never was an option for argparse.

This PR looks good to me. @gregvonkuster you have tested it I assume? @mvdbeek any comments on this one? If you agree I would propose to merge it and then move this file back to https://github.com/galaxyproject/ephemeris and depend with this role on https://github.com/galaxyproject/ephemeris. I will be back for one week but can do this afterwards.

mvdbeek commented 8 years ago

Yes, that's what im thinking. Im just surprised that specifying the panel labels works for me. Ephemeris already is a bit ahead of this script, I'll merge them.

On Aug 26, 2016 8:47 AM, "Björn Grüning" notifications@github.com wrote:

I'm really wondering why this is in the docs: https://github.com/bgruening/galaxy-metagenomics/blob/ master/metagenomics.yaml#L17 It seems it was never used and never was an option for argparse.

This PR looks good to me. @gregvonkuster https://github.com/gregvonkuster you have tested it I assume? @mvdbeek https://github.com/mvdbeek any comments on this one? If you agree I would propose to merge it and then move this file back to https://github.com/galaxyproject/ephemeris and depend with this role on https://github.com/galaxyproject/ephemeris. I will be back for one week but can do this afterwards.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/galaxyproject/ansible-galaxy-tools/pull/34#issuecomment-242644236, or mute the thread https://github.com/notifications/unsubscribe-auth/AGfVpZH5TeHhAFPGF5XKRMcbn3JstnzWks5qjowGgaJpZM4JtcIU .

bgruening commented 8 years ago

Good to hear that! I could have sworn that this worked for me as well :)

mvdbeek commented 8 years ago

I'm not too happy calling the command line option new_tool_panel_section_label and expecting this when parsing the tool_list.yml files. This implies you can only use this for new section labels, while you can use this to put tools into sections; if the section doesn't exist, it will be created. I think this is what a naive person would expect.

I think we should change this on the bioblend and galaxy side. Perhaps even eliminating tool_panel_section_id, since we can look up the id if we know the section label.

In the meantime, I don't understand why in the current situation this works for me. I have just confirmed it again ... bioblend should be raising an error, but things are fine.

So what I'm proposing is that in the last dictionary that gets passed on to bioblend, I change panel_section_label to new_panel_section_label. The additional commandline flag is already in ephemeris.

@gregvonkuster can you try installing ephemeris (pip install https://github.com/galaxyproject/ephemeris/archive/master.zip) and confirm this works and would be OK for you from the user experience side?

shed-install -g <galaxy_ip> -a <admin_key> --name bowtie2 --owner devteam --section_label bowtie2 --install_resolver_dependencies

Installs bowtie2 into the (new) bowtie2 section.

shed-install -g <galaxy_ip> -a <admin_key> --name hisat2 --owner iuc --section_label bowtie2 --install_resolver_dependencies

Install hisat2 into the previously created bowtie2 section

mvdbeek commented 8 years ago

Also, I know now why this currently works ... in the install_repository_revision function we pass only the dictionary values on to bioblend, so we're not making use of the keyword arguments.

gregvonkuster commented 8 years ago

I was not even aware of this repo https://github.com/galaxyproject/ephemeris, but watching it now. The reason I submitted this PR was because I was writing my Dockerfiles for installing tools using this approach, which I recently discovered must have been deprecated, and the new .yaml files should be used:

RUN install-repository \
...
   "--url https://toolshed.g2.bx.psu.edu/ -o iuc --name icqsol_add_surface_field_from_expression --panel-section-name Constructive Solid Geometry" \
   "--url https://toolshed.g2.bx.psu.edu/ -o iuc --name icqsol_color_surface_field --panel-section-id constructive_solid_geometry" \
   "--url https://toolshed.g2.bx.psu.edu/ -o iuc --name icqsol_compose_shapes --panel-section-id constructive_solid_geometry" \

When I changed my docker file to use the new .yaml approach, the entry for creating the new tool panel section did not work, and the tool was not even installed:

- name: icqsol_add_surface_field_from_expression
 owner: iuc
 new_tool_panel_section_label: "Constructive Solid Geometry"
 tool_shed_url: https://toolshed.g2.bx.psu.edu
 revisions:
   - e12b55e960de

- name: icqsol_add_texture
 owner: iuc
 tool_panel_section_id: "constructive_solid_geometry"
 tool_shed_url: https://toolshed.g2.bx.psu.edu
 revisions:
   - d6bbaddfa5af

I also tried this approach, and the tool was now installed, but all tools were installed outside of the desired tool panel section since it did not exist:

- name: icqsol_add_surface_field_from_expression
 owner: iuc
 tool_panel_section_label: "Constructive Solid Geometry"
 tool_shed_url: https://toolshed.g2.bx.psu.edu
 revisions:
   - e12b55e960de

I saw that @bgruening is using the ansible-galaxy-tools playbook here: https://github.com/bgruening/docker-galaxy-stable/blob/master/galaxy/install_tools_wrapper.sh#L50, so I looked at what may be causing the above problem, resulting in this PR.

If I am doing something wrong here with my Dockerfile (https://github.com/gregvonkuster/docker-galaxy-csg/blob/master/Dockerfile), please let me know. I'm happy to close this PR if it is really not needed.

I haven't done a lot of testing with this - it seems there are more repositories that do tool shed installs that I'd realized. If this PR is indeed useful, let me know a specific test I should perform and I'll do it. Is the test described by @mvdbeek above using ephemeris sufficient?

mvdbeek commented 8 years ago

This is all in a state of flux right now, I'm sorry you got trapped in this :(. And there was definitely an issue at hand!

I suspect that the ansible-galaxy-tools role in Bjoern's image is not using ephemeris, but indeed this script. And since on install time this role will install bioblend, you get a newer bioblend that has a different number of arguments (as compared to what the ansible-galaxy-tools role had been tested with), causing the behaviour you are seeing.

I'll fix up ephemeris, than this role and then we check how this will fit in Bjoern's dev branch?

gregvonkuster commented 8 years ago

Sound's great - let me know if I can help.

afgane commented 8 years ago

So what's the status now between shed_tools.py in ephemeris repo and install_tool_shed_tools.py from the ansible-galaxy-tools repo? Feels we're already out of sync between the two while they exist for basically the same purpose.

mvdbeek commented 8 years ago

Yes, I'll fix up ephemeris in https://github.com/galaxyproject/ephemeris/pull/11, then (I hope that) @jmchilton can do another ephemeris release, and then we should be able to merge https://github.com/galaxyproject/ansible-galaxy-tools/pull/33

gregvonkuster commented 8 years ago

@mvdbeek Should I close this PR due to your work here? https://github.com/bgruening/docker-galaxy-stable/pull/233

mvdbeek commented 8 years ago

Yep, we can always re-open if there are issues.

gregvonkuster commented 8 years ago

ok thanks!