Exception handling improvements.

For shed operations - catch exceptions, log them, but continue to completion and then return a 254 exit code at the end indicating there were problems with one or more repositories. This behavior has can be disabled with any of the shed operations using the new --fail_fast option.

Clean up noisy exception reported by @peterjc @ galaxyproject#158 (comment) (unless --fail_fast is used - then you get the full stack trace).

Tests.

Fixes galaxyproject#114.

planemo/commands/cmd_shed_diff.py

@@ -16,6 +16,7 @@
 @options.shed_owner_option()
 @options.shed_name_option()
 @options.shed_target_option()
+@options.shed_fail_fast_option()
 @click.option(
     "-o", "--output",
     type=click.Path(file_okay=True, resolve_path=True),

planemo/commands/cmd_shed_download.py

@@ -32,6 +32,7 @@
 @options.shed_name_option()
 @options.shed_target_option()
 @options.recursive_shed_option()
+@options.shed_fail_fast_option()
 @pass_context
 def cli(ctx, path, **kwds):
     """Download a tool repository as a tarball from the tool shed and extract

planemo/commands/cmd_shed_lint.py

@@ -19,6 +19,7 @@
 )
 @options.lint_xsd_option()
 @options.recursive_shed_option()
+@options.shed_fail_fast_option()
 @pass_context
 def cli(ctx, path, **kwds):
     """Check a Tool Shed repository for common problems.

planemo/commands/cmd_shed_upload.py

@@ -49,6 +49,7 @@
     default=False
 )
 @options.recursive_shed_option()
+@options.shed_fail_fast_option()
 @pass_context
 def cli(ctx, path, **kwds):
     """Handle possible recursion through paths for uploading files to a toolshed

planemo/options.py

@@ -272,6 +272,16 @@ def shed_password_option():
     )
 
 
+def shed_fail_fast_option():
+    return click.option(
+        '--fail_fast',
+        is_flag=True,
+        default=False,
+        help="If multiple repositories are specified and an error occurs "
+             "stop immediately instead of processing remaining repositories."
+    )
+
+
 def lint_xsd_option():
     return click.option(
         '--xsd',

planemo/shed/__init__.py

@@ -53,6 +53,8 @@
                               "repositories in .shed.yml at this time.")
 AUTO_NAME_CONFLICT_MESSAGE = ("Cannot specify both auto_tool_repositories and "
                               "in .shed.yml and --name on the command-line.")
+REALIZAION_PROBLEMS_MESSAGE = ("Problem encountered executing action for one or more "
+                               "repositories.")
 # Planemo generated or consumed files that do not need to be uploaded to the
 # tool shed.
 PLANEMO_FILES = [
@@ -487,10 +489,15 @@ def build_tarball(realized_path, **kwds):
 def for_each_repository(function, path, **kwds):
     ret_codes = []
     effective_repositories = realize_effective_repositories(path, **kwds)
-    for realized_repository in effective_repositories:
-        ret_codes.append(
-            function(realized_repository)
-        )
+    try:
+        for realized_repository in effective_repositories:
+            ret_codes.append(
+                function(realized_repository)
+            )
+    except RealizationException:
+        error(REALIZAION_PROBLEMS_MESSAGE)
+        return 254
+
     # "Good" returns are Nones, everything else is a -1 and should be
     # passed upwards.
     return 0 if all((not x) for x in ret_codes) else -1
@@ -528,16 +535,29 @@ def realize_effective_repositories(path, **kwds):
     tool).
     """
     raw_repo_objects = _find_raw_repositories(path, **kwds)
+    failed = False
     temp_directory = mkdtemp()
     try:
         for raw_repo_object in raw_repo_objects:
-            for realized_repo in raw_repo_object.realizations(
+            if isinstance(raw_repo_object, Exception):
+                _handle_realization_error(raw_repo_object, **kwds)
+                failed = True
+                continue
+
+            realized_repos = raw_repo_object.realizations(
                 temp_directory,
                 kwds.get("fail_on_missing", True)
-            ):
+            )
+            for realized_repo in realized_repos:
+                if isinstance(realized_repo, Exception):
+                    _handle_realization_error(realized_repo, **kwds)
+                    failed = True
+                    continue
                 yield realized_repo
     finally:
         shutil.rmtree(temp_directory)
+    if failed:
+        raise RealizationException()
 
 
 def _create_shed_config(ctx, path, **kwds):
@@ -615,7 +635,12 @@ def _find_raw_repositories(path, **kwds):
 
     config_name = None
     if len(shed_file_dirs) == 1:
-        config = shed_repo_config(shed_file_dirs[0], name=name)
+        shed_file_dir = shed_file_dirs[0]
+        try:
+            config = shed_repo_config(shed_file_dir, name=name)
+        except Exception as e:
+            error_message = PARSING_PROBLEM % (shed_file_dir, e)
+            return [RuntimeError(error_message)]
         config_name = config.get("name", None)
 
     if len(shed_file_dirs) > 1 and name is not None:
@@ -637,19 +662,18 @@ def _build_raw_repo_objects(raw_dirs, **kwds):
     """
     multiple = len(raw_dirs) > 1
     name = kwds.get("name", None)
-    skip_errors = kwds.get("skip_errors", False)
 
+    # List of RawRepositoryDirectories or parsing failures if
+    # fail_fast is not enabled.
     raw_repo_objects = []
     for raw_dir in raw_dirs:
         try:
             config = shed_repo_config(raw_dir, name=name)
         except Exception as e:
-            if skip_errors:
-                error_message = PARSING_PROBLEM % (raw_dir, e)
-                error(error_message)
-                continue
-            else:
-                raise
+            error_message = PARSING_PROBLEM % (raw_dir, e)
+            exception = RuntimeError(error_message)
+            _handle_realization_error(exception, **kwds)
+            raw_repo_objects.append(exception)
         raw_repo_object = RawRepositoryDirectory(raw_dir, config, multiple)
         raw_repo_objects.append(raw_repo_object)
     return raw_repo_objects
@@ -709,7 +733,7 @@ def _realize_to(self, directory, name, multiple, fail_on_missing):
         missing = realized_files.include_failures
         if missing and fail_on_missing:
             msg = "Failed to include files for %s" % missing
-            raise Exception(msg)
+            return RuntimeError(msg)
 
         for realized_file in realized_files.files:
             relative_dest = realized_file.relative_dest
@@ -918,6 +942,14 @@ def _shed_config_excludes(config):
     return config.get('ignore', []) + config.get('exclude', [])
 
 
+def _handle_realization_error(exception, **kwds):
+    fail_fast = kwds.get("fail_fast", False)
+    if fail_fast:
+        raise exception
+    else:
+        error(str(exception))
+
+
 def validate_repo_name(name):
     def _build_error(descript):
         return "Repository name [%s] invalid. %s" % (name, descript)
@@ -956,6 +988,12 @@ def _build_error(descript):
     return msg
 
 
+class RealizationException(Exception):
+    """ This exception indicates there was a problem while
+    realizing effective repositories for a shed command. As a
+    precondition - the user has already been informed with error().
+    """
+
 __all__ = [
     'for_each_repository',
     'api_exception_to_message',

planemo/shed_lint.py

@@ -152,6 +152,7 @@ def lint_shed_yaml(path, lint_ctx):
         shed_contents = yaml.load(open(shed_yaml, "r"))
     except Exception as e:
         lint_ctx.warn("Failed to parse .shed.yml file [%s]" % str(e))
+        return
     lint_ctx.info(".shed.yml found and appears to be valid YAML.")
     _lint_shed_contents(lint_ctx, path, shed_contents)
 

tests/data/repos/bad_invalid_yaml/.shed.yml

@@ -0,0 +1,8 @@
+name bad_missing_include
+owner: iuc
+description: A single tool with an ignore.
+include: 
+ - related_file
+ - cat.xml
+ - test-data/**
+

tests/data/repos/bad_invalid_yaml/cat.xml

@@ -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>

tests/data/repos/bad_invalid_yaml/related_file

@@ -0,0 +1,2 @@
+A related non-tool file.
+

tests/data/repos/bad_invalid_yaml/test-data/1.bed

@@ -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	-

tests/data/repos/bad_invalid_yaml/test-data/1_bed_random_lines_1_seed_asdf_out.bed

@@ -0,0 +1 @@
+chr5	131424298	131424460	CCDS4149.1_cds_0_0_chr5_131424299_f	0	+

tests/test_shed_lint.py

@@ -1,4 +1,4 @@
-
+from os.path import join
 from .test_utils import CliTestCase
 
 
@@ -26,3 +26,26 @@ def test_invalid_repos(self):
             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)
+        with self._isolate_repo("bad_invalid_yaml"):
+            self._check_exit_code(["shed_lint"], exit_code=254)
+
+    def test_invalid_nested(self):
+        # Created a nested repository with one good and one
+        # invalid repository and make sure it runs and produces
+        # a 254 (it ran to completion but one or more things failed
+        # )
+        with self._isolate() as f:
+            for name in ["bad_invalid_yaml", "single_tool_exclude"]:
+                self._copy_repo(name, join(f, name))
+                self._copy_repo(name, join(f, name))
+            self._check_exit_code(["shed_lint", "-r"], exit_code=254)
+
+    def test_fail_fast(self):
+        # Created a nested repository with one good and one
+        # invalid repository and make sure it runs and produces
+        # a 254 (it ran to completion but one or more things failed
+        # )
+        with self._isolate_repo("bad_invalid_yaml"):
+            r = self._check_exit_code(["shed_lint", "--fail_fast"],
+                                      exit_code=-1)
+            assert isinstance(r.exception, RuntimeError)

tests/test_shed_upload.py

@@ -52,7 +52,7 @@ 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)
+            self._check_exit_code(upload_command, exit_code=254)
 
     def test_upload_recusrive(self):
         with self._isolate_repo("multi_repos_nested") as f: