Skip to content

Commit

Permalink
Fail if missing inclusions encountered.
Browse files Browse the repository at this point in the history
- Shed actions (e.g. upload) will fail for that repository.
- Shed linting will warn about this.

Fixes #158 reported by @peterjc.
  • Loading branch information
jmchilton committed May 1, 2015
1 parent 01ddcc2 commit 4cb024b
Show file tree
Hide file tree
Showing 10 changed files with 156 additions and 15 deletions.
4 changes: 2 additions & 2 deletions planemo/commands/cmd_shed_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ def cli(ctx, path, **kwds):
"""Check a Tool Shed repository for common problems.
"""
def lint(realized_repository):
path = realized_repository.real_path
return shed_lint.lint_repository(ctx, path, **kwds)
return shed_lint.lint_repository(ctx, realized_repository, **kwds)

kwds["fail_on_missing"] = False
exit_code = shed.for_each_repository(lint, path, **kwds)
sys.exit(exit_code)
40 changes: 28 additions & 12 deletions planemo/shed/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from collections import namedtuple
import copy
import fnmatch
import hashlib
Expand Down Expand Up @@ -530,7 +531,10 @@ def realize_effective_repositories(path, **kwds):
temp_directory = mkdtemp()
try:
for raw_repo_object in raw_repo_objects:
for realized_repo in raw_repo_object.realizations(temp_directory):
for realized_repo in raw_repo_object.realizations(
temp_directory,
kwds.get("fail_on_missing", True)
):
yield realized_repo
finally:
shutil.rmtree(temp_directory)
Expand Down Expand Up @@ -684,24 +688,30 @@ def __init__(self, path, config, multiple):
def _hash(self, name):
return hashlib.md5(name.encode('utf-8')).hexdigest()

def realizations(self, parent_directory):
def realizations(self, parent_directory, fail_on_missing=True):
names = self._repo_names()

for name in names:
directory = os.path.join(parent_directory, self._hash(name), name)
multiple = self.multiple or len(names) > 1
if not os.path.exists(directory):
os.makedirs(directory)
yield self._realize_to(directory, name, multiple)
yield self._realize_to(directory, name, multiple, fail_on_missing)

def _realize_to(self, directory, name, multiple):
def _realize_to(self, directory, name, multiple, fail_on_missing):
ignore_list = []
config = self._realize_config(name)
excludes = _shed_config_excludes(config)
for exclude in excludes:
ignore_list.extend(_glob(self.path, exclude))

for realized_file in self._realized_files(name):
realized_files = self._realized_files(name)
missing = realized_files.include_failures
if missing and fail_on_missing:
msg = "Failed to include files for %s" % missing
raise Exception(msg)

This comment has been minimized.

Copy link
@peterjc

peterjc May 1, 2015

Contributor

Does planemo have its own exception classes? Otherwise maybe ValueError or RuntimeError might be better here?

This comment has been minimized.

Copy link
@jmchilton

jmchilton May 1, 2015

Author Member

It doesn't really have any - sometimes it uses click's UsageError but that isn't relevant here. I rarely use ValueError and have never thrown a RuntimeError - but reading through the desciptions it seems like I should be using at least RuntimeError a lot more frequently.

Do you have a preference between ValueError and RuntimeError in this scenario?
#172


for realized_file in realized_files.files:
relative_dest = realized_file.relative_dest
implicit_ignore = self._implicit_ignores(relative_dest)
explicit_ignore = (realized_file.absolute_src in ignore_list)
Expand All @@ -718,7 +728,8 @@ def _realize_to(self, directory, name, multiple):
realized_path=directory,
real_path=self.path,
config=config,
multiple=multiple
multiple=multiple,
missing=missing,
)

def _repo_names(self):
Expand All @@ -727,12 +738,14 @@ def _repo_names(self):
def _realized_files(self, name):
config = self._realize_config(name)
realized_files = []
missing = []
for include in config["include"]:
realized_files.extend(
RealizedFile.realized_files_for(self.path, include)
)
for realized_file in realized_files:
yield realized_file
included = RealizedFile.realized_files_for(self.path, include)
if not included:
missing.append(include)
else:
realized_files.extend(included)
return RealizedFiles(realized_files, missing)

def _realize_config(self, name):
config = copy.deepcopy(self.config)
Expand Down Expand Up @@ -761,6 +774,8 @@ def _implicit_ignores(self, relative_path):
return True
return False

RealizedFiles = namedtuple("RealizedFiles", ["files", "include_failures"])


class RealizedFile(object):

Expand Down Expand Up @@ -833,12 +848,13 @@ def __str__(self):

class RealizedRepositry(object):

def __init__(self, realized_path, real_path, config, multiple):
def __init__(self, realized_path, real_path, config, multiple, missing):
self.path = realized_path
self.real_path = real_path
self.config = config
self.name = config["name"]
self.multiple = multiple
self.missing = missing

def pattern_to_file_name(self, pattern):
if not self.multiple:
Expand Down
19 changes: 18 additions & 1 deletion planemo/shed_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,18 @@
]


def lint_repository(ctx, path, **kwds):
def lint_repository(ctx, realized_repository, **kwds):
# TODO: this really needs to start working with realized path.
path = realized_repository.real_path
info("Linting repository %s" % path)
lint_args = build_lint_args(ctx, **kwds)
lint_ctx = LintContext(lint_args["level"])
lint_ctx.lint(
"lint_expansion",
lint_expansion,
realized_repository,
)

lint_ctx.lint(
"lint_tool_dependencies",
lint_tool_dependencies,
Expand Down Expand Up @@ -74,6 +82,15 @@ def lint_repository(ctx, path, **kwds):
return 1 if failed else 0


def lint_expansion(realized_repository, lint_ctx):
missing = realized_repository.missing
if missing:
msg = "Failed to expand inclusions %s" % missing
lint_ctx.warn(msg)
else:
lint_ctx.info("Included files all found.")


def lint_readme(path, lint_ctx):
readme_rst = os.path.join(path, "README.rst")
readme = os.path.join(path, "README")
Expand Down
9 changes: 9 additions & 0 deletions tests/data/repos/bad_missing_include/.shed.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
name: bad_missing_include
owner: iuc
description: A single tool with an ignore.
include:
- related_file
- cat.xml
- test-data/**
- my_missing_file

23 changes: 23 additions & 0 deletions tests/data/repos/bad_missing_include/cat.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<tool id="cat" name="Concatenate datasets (for test workflows)" version="1.0">
<description>tail-to-head</description>
<command>
cat $input1 #for $q in $queries# ${q.input2} #end for# > $out_file1
</command>
<inputs>
<param name="input1" type="data" label="Concatenate Dataset"/>
<repeat name="queries" title="Dataset">
<param name="input2" type="data" label="Select" />
</repeat>
</inputs>
<outputs>
<data name="out_file1" format="input" metadata_source="input1"/>
</outputs>
<tests>
<test>
<param name="input1" value="1.bed"/>
<output name="out_file1" file="1.bed"/>
</test>
</tests>
<help>
</help>
</tool>
2 changes: 2 additions & 0 deletions tests/data/repos/bad_missing_include/related_file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
A related non-tool file.

65 changes: 65 additions & 0 deletions tests/data/repos/bad_missing_include/test-data/1.bed
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
chr1 147962192 147962580 CCDS989.1_cds_0_0_chr1_147962193_r 0 -
chr1 147984545 147984630 CCDS990.1_cds_0_0_chr1_147984546_f 0 +
chr1 148078400 148078582 CCDS993.1_cds_0_0_chr1_148078401_r 0 -
chr1 148185136 148185276 CCDS996.1_cds_0_0_chr1_148185137_f 0 +
chr10 55251623 55253124 CCDS7248.1_cds_0_0_chr10_55251624_r 0 -
chr11 116124407 116124501 CCDS8374.1_cds_0_0_chr11_116124408_r 0 -
chr11 116206508 116206563 CCDS8377.1_cds_0_0_chr11_116206509_f 0 +
chr11 116211733 116212337 CCDS8378.1_cds_0_0_chr11_116211734_r 0 -
chr11 1812377 1812407 CCDS7726.1_cds_0_0_chr11_1812378_f 0 +
chr12 38440094 38440321 CCDS8736.1_cds_0_0_chr12_38440095_r 0 -
chr13 112381694 112381953 CCDS9526.1_cds_0_0_chr13_112381695_f 0 +
chr14 98710240 98712285 CCDS9949.1_cds_0_0_chr14_98710241_r 0 -
chr15 41486872 41487060 CCDS10096.1_cds_0_0_chr15_41486873_r 0 -
chr15 41673708 41673857 CCDS10097.1_cds_0_0_chr15_41673709_f 0 +
chr15 41679161 41679250 CCDS10098.1_cds_0_0_chr15_41679162_r 0 -
chr15 41826029 41826196 CCDS10101.1_cds_0_0_chr15_41826030_f 0 +
chr16 142908 143003 CCDS10397.1_cds_0_0_chr16_142909_f 0 +
chr16 179963 180135 CCDS10401.1_cds_0_0_chr16_179964_r 0 -
chr16 244413 244681 CCDS10402.1_cds_0_0_chr16_244414_f 0 +
chr16 259268 259383 CCDS10403.1_cds_0_0_chr16_259269_r 0 -
chr18 23786114 23786321 CCDS11891.1_cds_0_0_chr18_23786115_r 0 -
chr18 59406881 59407046 CCDS11985.1_cds_0_0_chr18_59406882_f 0 +
chr18 59455932 59456337 CCDS11986.1_cds_0_0_chr18_59455933_r 0 -
chr18 59600586 59600754 CCDS11988.1_cds_0_0_chr18_59600587_f 0 +
chr19 59068595 59069564 CCDS12866.1_cds_0_0_chr19_59068596_f 0 +
chr19 59236026 59236146 CCDS12872.1_cds_0_0_chr19_59236027_r 0 -
chr19 59297998 59298008 CCDS12877.1_cds_0_0_chr19_59297999_f 0 +
chr19 59302168 59302288 CCDS12878.1_cds_0_0_chr19_59302169_r 0 -
chr2 118288583 118288668 CCDS2120.1_cds_0_0_chr2_118288584_f 0 +
chr2 118394148 118394202 CCDS2121.1_cds_0_0_chr2_118394149_r 0 -
chr2 220190202 220190242 CCDS2441.1_cds_0_0_chr2_220190203_f 0 +
chr2 220229609 220230869 CCDS2443.1_cds_0_0_chr2_220229610_r 0 -
chr20 33330413 33330423 CCDS13249.1_cds_0_0_chr20_33330414_r 0 -
chr20 33513606 33513792 CCDS13255.1_cds_0_0_chr20_33513607_f 0 +
chr20 33579500 33579527 CCDS13256.1_cds_0_0_chr20_33579501_r 0 -
chr20 33593260 33593348 CCDS13257.1_cds_0_0_chr20_33593261_f 0 +
chr21 32707032 32707192 CCDS13614.1_cds_0_0_chr21_32707033_f 0 +
chr21 32869641 32870022 CCDS13615.1_cds_0_0_chr21_32869642_r 0 -
chr21 33321040 33322012 CCDS13620.1_cds_0_0_chr21_33321041_f 0 +
chr21 33744994 33745040 CCDS13625.1_cds_0_0_chr21_33744995_r 0 -
chr22 30120223 30120265 CCDS13897.1_cds_0_0_chr22_30120224_f 0 +
chr22 30160419 30160661 CCDS13898.1_cds_0_0_chr22_30160420_r 0 -
chr22 30665273 30665360 CCDS13901.1_cds_0_0_chr22_30665274_f 0 +
chr22 30939054 30939266 CCDS13903.1_cds_0_0_chr22_30939055_r 0 -
chr5 131424298 131424460 CCDS4149.1_cds_0_0_chr5_131424299_f 0 +
chr5 131556601 131556672 CCDS4151.1_cds_0_0_chr5_131556602_r 0 -
chr5 131621326 131621419 CCDS4152.1_cds_0_0_chr5_131621327_f 0 +
chr5 131847541 131847666 CCDS4155.1_cds_0_0_chr5_131847542_r 0 -
chr6 108299600 108299744 CCDS5061.1_cds_0_0_chr6_108299601_r 0 -
chr6 108594662 108594687 CCDS5063.1_cds_0_0_chr6_108594663_f 0 +
chr6 108640045 108640151 CCDS5064.1_cds_0_0_chr6_108640046_r 0 -
chr6 108722976 108723115 CCDS5067.1_cds_0_0_chr6_108722977_f 0 +
chr7 113660517 113660685 CCDS5760.1_cds_0_0_chr7_113660518_f 0 +
chr7 116512159 116512389 CCDS5771.1_cds_0_0_chr7_116512160_r 0 -
chr7 116714099 116714152 CCDS5773.1_cds_0_0_chr7_116714100_f 0 +
chr7 116945541 116945787 CCDS5774.1_cds_0_0_chr7_116945542_r 0 -
chr8 118881131 118881317 CCDS6324.1_cds_0_0_chr8_118881132_r 0 -
chr9 128764156 128764189 CCDS6914.1_cds_0_0_chr9_128764157_f 0 +
chr9 128787519 128789136 CCDS6915.1_cds_0_0_chr9_128787520_r 0 -
chr9 128882427 128882523 CCDS6917.1_cds_0_0_chr9_128882428_f 0 +
chr9 128937229 128937445 CCDS6919.1_cds_0_0_chr9_128937230_r 0 -
chrX 122745047 122745924 CCDS14606.1_cds_0_0_chrX_122745048_f 0 +
chrX 152648964 152649196 CCDS14733.1_cds_0_0_chrX_152648965_r 0 -
chrX 152691446 152691471 CCDS14735.1_cds_0_0_chrX_152691447_f 0 +
chrX 152694029 152694263 CCDS14736.1_cds_0_0_chrX_152694030_r 0 -
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
chr5 131424298 131424460 CCDS4149.1_cds_0_0_chr5_131424299_f 0 +
2 changes: 2 additions & 0 deletions tests/test_shed_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ def test_invalid_repos(self):
self._check_exit_code(["shed_lint"], exit_code=-1)
with self._isolate_repo("bad_repo_name"):
self._check_exit_code(["shed_lint"], exit_code=-1)
with self._isolate_repo("bad_missing_include"):
self._check_exit_code(["shed_lint"], exit_code=-1)
6 changes: 6 additions & 0 deletions tests/test_shed_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ def test_cannont_recreate(self):
self._check_exit_code(create_command)
self._check_exit_code(create_command, exit_code=-1)

def test_cannot_upload_missing_include(self):
with self._isolate_repo("bad_missing_include"):
upload_command = ["shed_upload", "--tar_only"]
upload_command.extend(self._shed_args())
self._check_exit_code(upload_command, exit_code=-1)

def test_upload_recusrive(self):
with self._isolate_repo("multi_repos_nested") as f:
upload_command = [
Expand Down

0 comments on commit 4cb024b

Please sign in to comment.