Skip to content

Commit 912df02

Browse files
committed
Use XSD to lint tools and repositories.
- Implement validation abstraction that will use either lxml (Python lib) or xmllint (command line app) dependending on what is available - with test cases. - Add a ``--xsd`` flag to the lint command that lints against the experimental XSD from https://github.com/JeanFred/Galaxy-XSD. - Implement ``repository_dependencies.xsd`` to describe Tool Shed ``repository_dependencies.xml`` files (fairly complete). - Implement ``tool_dependencies.xsd`` to describe Tool Shed ``tool_dependencies.xml`` files. - Validates attributes and elements down to the ``action`` elements and then largely gives up (sticking ``any`` and ``anyAttribute`` tags on that element). - Registers everything in tools-devteam as valid, and detects one invalid XML file in tools-iuc. - Implement new ``shed_lint`` command that: - Validates tool_dependencies.xml against schema - Validates repository_dependencies.xml against schema - Bare minimum to lint .shed.yml files. - Optionally also lints tools in repsitories with the ``--tools`` argument. - Can recursively lint many repositories at ont time ``-r``. - Refactoring of existing stuff to support this and make room for XSD validation of tool XML files and generalizing applying actions over many repositories and many tools.
1 parent 0820f48 commit 912df02

34 files changed

+2428
-72
lines changed

planemo/commands/cmd_lint.py

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,24 @@
11
import sys
2-
import traceback
2+
33
import click
44

55
from planemo.cli import pass_context
6-
from planemo.io import info
7-
from planemo.io import error
86
from planemo import options
97

10-
from galaxy.tools.loader_directory import load_tool_elements_from_path
11-
from galaxy.tools.lint import lint_xml
12-
13-
SKIP_XML_MESSAGE = "Skipping XML file - does not appear to be a tool %s."
14-
LINTING_TOOL_MESSAGE = "Linting tool %s"
8+
from planemo.tool_lint import build_lint_args
9+
from planemo.tool_lint import lint_tools_on_path
1510

1611

1712
@click.command('lint')
1813
@options.optional_tools_arg()
19-
@click.option(
20-
'--report_level',
21-
type=click.Choice(['all', 'warn', 'error']),
22-
default="all"
23-
)
24-
@click.option(
25-
'--fail_level',
26-
type=click.Choice(['warn', 'error']),
27-
default="warn"
28-
)
14+
@options.report_level_option()
15+
@options.fail_level_option()
16+
@options.lint_xsd_option()
2917
@pass_context
30-
def cli(ctx, path, report_level="all", fail_level="warn"):
18+
def cli(ctx, path, **kwds):
3119
"""Check specified tool(s) for common errors and adherence to best
3220
practices.
3321
"""
34-
exit = 0
35-
lint_args = dict(level=report_level, fail_level=fail_level)
36-
tools = load_tool_elements_from_path(path, load_exception_handler)
37-
valid_tools = 0
38-
for (tool_path, tool_xml) in tools:
39-
if tool_xml.getroot().tag != "tool":
40-
if ctx.verbose:
41-
info(SKIP_XML_MESSAGE % tool_path)
42-
continue
43-
info("Linting tool %s" % tool_path)
44-
if not lint_xml(tool_xml, **lint_args):
45-
exit = 1
46-
else:
47-
valid_tools += 1
48-
if exit == 0 and valid_tools == 0:
49-
exit = 2
22+
lint_args = build_lint_args(**kwds)
23+
exit = lint_tools_on_path(ctx, path, lint_args)
5024
sys.exit(exit)
51-
52-
53-
def load_exception_handler(path, exc_info):
54-
error("Error loading tool with path %s" % path)
55-
traceback.print_exception(*exc_info, limit=1, file=sys.stderr)

planemo/commands/cmd_shed_lint.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import click
2+
import sys
3+
4+
from planemo.cli import pass_context
5+
from planemo import options
6+
from planemo import shed
7+
from planemo import shed_lint
8+
9+
10+
@click.command('shed_lint')
11+
@options.optional_project_arg(exists=True)
12+
@options.report_level_option()
13+
@options.fail_level_option()
14+
@options.click.option(
15+
'--tools',
16+
is_flag=True,
17+
default=False,
18+
help=("Lint tools discovered in the process of linting repositories.")
19+
)
20+
@options.lint_xsd_option()
21+
@options.recursive_shed_option()
22+
@pass_context
23+
def cli(ctx, path, recursive=False, **kwds):
24+
"""Check a Tool Shed repository for common problems.
25+
"""
26+
def lint(path):
27+
return shed_lint.lint_repository(ctx, path, **kwds)
28+
29+
if recursive:
30+
exit_code = shed.for_each_repository(lint, path)
31+
else:
32+
exit_code = lint(path)
33+
34+
sys.exit(exit_code)

planemo/commands/cmd_shed_upload.py

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
from planemo.io import shell
1111
import json
1212

13-
import fnmatch
14-
import os
1513

1614
tar_path = click.Path(
1715
exists=True,
@@ -54,15 +52,15 @@
5452
is_flag=True,
5553
default=False
5654
)
57-
@click.option(
58-
'-r', '--recursive',
59-
is_flag=True,
60-
help="Recursively search for repositories to publish to a tool shed",
61-
)
55+
@options.recursive_shed_option()
6256
@pass_context
6357
def cli(ctx, path, **kwds):
6458
"""Handle possible recursion through paths for uploading files to a toolshed
6559
"""
60+
61+
def upload(path):
62+
return __handle_upload(ctx, **kwds)
63+
6664
if kwds['recursive']:
6765
if kwds['name'] is not None:
6866
error("--name is incompatible with --recursive")
@@ -71,17 +69,9 @@ def cli(ctx, path, **kwds):
7169
error("--tar is incompatible with --recursive")
7270
return -1
7371

74-
ret_codes = []
75-
for base_path, dirnames, filenames in os.walk(path):
76-
for filename in fnmatch.filter(filenames, '.shed.yml'):
77-
ret_codes.append(
78-
__handle_upload(ctx, base_path, **kwds)
79-
)
80-
# "Good" returns are Nones, everything else is a -1 and should be
81-
# passed upwards.
82-
return None if all(x is None for x in ret_codes) else -1
72+
return shed.for_each_repository(upload, path)
8373
else:
84-
return __handle_upload(ctx, path, **kwds)
74+
return upload(path)
8575

8676

8777
def __handle_upload(ctx, path, **kwds):
@@ -121,6 +111,7 @@ def __handle_upload(ctx, path, **kwds):
121111
error(e2.read())
122112
return -1
123113
info("Repository %s updated successfully." % path)
114+
return 0
124115

125116

126117
def __find_repository(ctx, tsi, path, **kwds):

planemo/galaxy_test.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
""" Utilities for reasoning about Galaxy test results.
22
"""
3+
from __future__ import absolute_import
34
import json
45
import xml.etree.ElementTree as ET
56

planemo/lint.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import os
2+
3+
from planemo.xml import validation
4+
5+
6+
def lint_xsd(lint_ctx, schema_path, path):
7+
name = os.path.basename(path)
8+
validator = validation.get_validator(require=True)
9+
validation_result = validator.validate(schema_path, path)
10+
if not validation_result.passed:
11+
msg = "Invalid %s found. Errors [%s]"
12+
msg = msg % (name, validation_result.output)
13+
lint_ctx.error(msg)
14+
else:
15+
lint_ctx.info("%s found and appears to be valid XML" % name)

planemo/linters/__init__.py

Whitespace-only changes.

planemo/linters/xsd.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
""" Tool linting module that lints Galaxy tool against experimental XSD.
2+
"""
3+
import copy
4+
import os
5+
import tempfile
6+
7+
from planemo.xml import XSDS_PATH
8+
import planemo.lint
9+
10+
TOOL_XSD = os.path.join(XSDS_PATH, "tool", "galaxy.xsd")
11+
12+
13+
def lint_tool_xsd(root, lint_ctx):
14+
""" Write a temp file out and lint it.
15+
"""
16+
with tempfile.NamedTemporaryFile() as tf:
17+
_clean_root(root).write(tf.name)
18+
planemo.lint.lint_xsd(lint_ctx, TOOL_XSD, tf.name)
19+
20+
21+
def _clean_root(root):
22+
""" XSD assumes macros have been expanded, so remove them.
23+
"""
24+
clean_root = copy.deepcopy(root)
25+
to_remove = []
26+
for macros_el in clean_root.findall("macros"):
27+
to_remove.append(macros_el)
28+
for macros_el in to_remove:
29+
clean_root.getroot().remove(macros_el)
30+
return clean_root

planemo/options.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,3 +249,38 @@ def shed_password_option():
249249
help="Password for Tool Shed auth (required unless shed_key is "
250250
"specified)."
251251
)
252+
253+
254+
def lint_xsd_option():
255+
return click.option(
256+
'--xsd',
257+
is_flag=True,
258+
default=False,
259+
help=("Include experimental tool XSD validation in linting "
260+
"process (requires xmllint on PATH or lxml installed).")
261+
)
262+
263+
264+
def report_level_option():
265+
return click.option(
266+
'--report_level',
267+
type=click.Choice(['all', 'warn', 'error']),
268+
default="all",
269+
)
270+
271+
272+
def fail_level_option():
273+
return click.option(
274+
'--fail_level',
275+
type=click.Choice(['warn', 'error']),
276+
default="warn"
277+
)
278+
279+
280+
def recursive_shed_option():
281+
return click.option(
282+
'-r',
283+
'--recursive',
284+
is_flag=True,
285+
help="Recursively perform command for nested repository directories.",
286+
)

planemo/shed.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import fnmatch
2+
import glob
13
import os
2-
from tempfile import mkstemp
34
import tarfile
5+
from tempfile import mkstemp
6+
47
import yaml
5-
import glob
68

79
try:
810
from bioblend import toolshed
@@ -192,6 +194,24 @@ def build_tarball(tool_path):
192194
return temp_path
193195

194196

197+
def walk_repositories(path):
198+
""" Recurse through directories and find effective repositories. """
199+
for base_path, dirnames, filenames in os.walk(path):
200+
for filename in fnmatch.filter(filenames, '.shed.yml'):
201+
yield base_path
202+
203+
204+
def for_each_repository(function, path):
205+
ret_codes = []
206+
for base_path in walk_repositories(path):
207+
ret_codes.append(
208+
function(base_path)
209+
)
210+
# "Good" returns are Nones, everything else is a -1 and should be
211+
# passed upwards.
212+
return 0 if all((not x) for x in ret_codes) else -1
213+
214+
195215
def username(tsi):
196216
user = _user(tsi)
197217
return user["username"]

planemo/shed_lint.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import os
2+
import yaml
3+
from galaxy.tools.lint import LintContext
4+
from planemo.lint import lint_xsd
5+
from planemo.tool_lint import (
6+
build_lint_args,
7+
yield_tool_xmls,
8+
)
9+
from planemo.xml import XSDS_PATH
10+
11+
12+
from planemo.io import info
13+
from planemo.io import error
14+
15+
from galaxy.tools.lint import lint_xml_with
16+
17+
TOOL_DEPENDENCIES_XSD = os.path.join(XSDS_PATH, "tool_dependencies.xsd")
18+
REPO_DEPENDENCIES_XSD = os.path.join(XSDS_PATH, "repository_dependencies.xsd")
19+
20+
21+
def lint_repository(ctx, path, **kwds):
22+
info("Linting repository %s" % path)
23+
lint_args = build_lint_args(**kwds)
24+
lint_ctx = LintContext(lint_args["level"])
25+
lint_ctx.lint(
26+
"tool_dependencies",
27+
lint_tool_dependencies,
28+
path,
29+
)
30+
lint_ctx.lint(
31+
"repository_dependencies",
32+
lint_repository_dependencies,
33+
path,
34+
)
35+
lint_ctx.lint(
36+
"shed_yaml",
37+
lint_shed_yaml,
38+
path,
39+
)
40+
if kwds["tools"]:
41+
for (tool_path, tool_xml) in yield_tool_xmls(ctx, path):
42+
info("+Linting tool %s" % tool_path)
43+
lint_xml_with(
44+
lint_ctx,
45+
tool_xml,
46+
extra_modules=lint_args["extra_modules"]
47+
)
48+
failed = lint_ctx.failed(lint_args["fail_level"])
49+
if failed:
50+
error("Failed linting")
51+
return 1 if failed else 0
52+
53+
54+
def lint_tool_dependencies(path, lint_ctx):
55+
tool_dependencies = os.path.join(path, "tool_dependencies.xml")
56+
if not os.path.exists(tool_dependencies):
57+
lint_ctx.info("No tool_dependencies.xml, skipping.")
58+
return
59+
lint_xsd(lint_ctx, TOOL_DEPENDENCIES_XSD, tool_dependencies)
60+
61+
62+
def lint_repository_dependencies(path, lint_ctx):
63+
repo_dependencies = os.path.join(path, "repository_dependencies.xml")
64+
if not os.path.exists(repo_dependencies):
65+
lint_ctx.info("No repository_dependencies.xml, skipping.")
66+
return
67+
lint_xsd(lint_ctx, REPO_DEPENDENCIES_XSD, repo_dependencies)
68+
69+
70+
def lint_shed_yaml(path, lint_ctx):
71+
shed_yaml = os.path.join(path, ".shed.yml")
72+
if not os.path.exists(shed_yaml):
73+
lint_ctx.info("No .shed.yml file found, skipping.")
74+
return
75+
try:
76+
shed_contents = yaml.load(open(shed_yaml, "r"))
77+
except Exception as e:
78+
lint_ctx.warn("Failed to parse .shed.yml file [%s]" % str(e))
79+
80+
warned = False
81+
for required_key in ["owner", "name"]:
82+
if required_key not in shed_contents:
83+
lint_ctx.warn(".shed.yml did not contain key [%s]" % required_key)
84+
warned = True
85+
86+
if not warned:
87+
lint_ctx.info(".shed.yml found and appears to be valid YAML.")

0 commit comments

Comments
 (0)