Skip to content

Commit

Permalink
Exception handling improvements.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jmchilton committed May 1, 2015
1 parent 997ebce commit 1b0d417
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 17 deletions.
1 change: 1 addition & 0 deletions planemo/commands/cmd_shed_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions planemo/commands/cmd_shed_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions planemo/commands/cmd_shed_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions planemo/commands/cmd_shed_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions planemo/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
68 changes: 53 additions & 15 deletions planemo/shed/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions planemo/shed_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
25 changes: 24 additions & 1 deletion tests/test_shed_lint.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

from os.path import join
from .test_utils import CliTestCase


Expand Down Expand Up @@ -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)
2 changes: 1 addition & 1 deletion tests/test_shed_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 1b0d417

Please sign in to comment.