Skip to content

Add tool_deps tool to allow triggering installation of local tool dep…#130

Merged
mvdbeek merged 7 commits intogalaxyproject:masterfrom
brinkmanlab:local-tools
Jul 18, 2019
Merged

Add tool_deps tool to allow triggering installation of local tool dep…#130
mvdbeek merged 7 commits intogalaxyproject:masterfrom
brinkmanlab:local-tools

Conversation

@innovate-invent
Copy link
Copy Markdown
Contributor

…endencies

The contribution docs don't detail how to do tests. This tool requires that a tool has already been manually installed into galaxy.

It also isn't clear how to provide a command line wrapper to execute the python script

Copy link
Copy Markdown
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/ephemeris/tool_deps.py Outdated
Comment thread docs/commands.rst Outdated
Use nargs to accept multiple inputs
@mvdbeek
Copy link
Copy Markdown
Member

mvdbeek commented Jun 4, 2019

This requires either the tool xml file or a tool_conf.xml, neither of which are formats we deal with in ephemeris and that are very local to Galaxy. I'd suggest a simple yaml list of tool ids.

@innovate-invent
Copy link
Copy Markdown
Contributor Author

@mvdbeek It does take a list of tool ids on the command line. The tool_conf.xml support is useful when deploying a container and ephemeris is connecting to localhost with direct access to galaxies filesystem.

@mvdbeek
Copy link
Copy Markdown
Member

mvdbeek commented Jun 9, 2019

I understand the utility, but in principle that's not the scope of ephemeris. Still this mostly works via the API, so if you can add loading a plain list of tool ids via yaml.safe_load and make that the default for -t (in line with other commands expecting a yaml file with -t) I'm OK to merge this.

Comment thread docs/commands/install-tool-deps.rst Outdated
@mvdbeek
Copy link
Copy Markdown
Member

mvdbeek commented Jun 12, 2019

There are some linting errors now (https://travis-ci.org/galaxyproject/ephemeris/jobs/543810023#L475), the other failures should have been temporary and unrelated.

@innovate-invent
Copy link
Copy Markdown
Contributor Author

The current lint failure is not relevant to this project. The security implication is that it could fill RAM and crash the system if a malicious xml file is provided.

@mvdbeek mvdbeek merged commit e0a011b into galaxyproject:master Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants