Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore failure to download output datasets #1179

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Jul 12, 2021

If we raise an exception here we won't get a full report with what
really is the issue.
Also uses output_to_cwl_json for hdas, since it produces the same
output.

Should produce a more meaningful test report for galaxyproject/iwc#47

If we raise an exception here we won't get a full report with what
really is the issue.
Also uses output_to_cwl_json for hdas, since it produces the same
output.
@mvdbeek mvdbeek force-pushed the improve_error_reporting_failed_requests branch from 34eb422 to e22ddd3 Compare July 12, 2021 13:35
@mvdbeek mvdbeek requested a review from jmchilton July 12, 2021 15:53
# We don't want this to break the rest of the test report.
# Should probably find a way to propagate this back into the report.
ctx.vlog(f"Failed to download history content at URL {url}, exception: {e}")
return
Copy link
Member

@jmchilton jmchilton Jul 12, 2021

Choose a reason for hiding this comment

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

The job needs to fail right? You cannot run test assertions on this.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we're reaching this point the job has already failed (most likely), and the test assertions would run on an empty file, so they should still produce the failure. Checkout line 380 and 363, where we're also skipping outputs.

@jmchilton jmchilton merged commit e229a35 into galaxyproject:master Jul 12, 2021
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.

2 participants