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

JobWrapper: use correct fail method #14210

Closed

Conversation

bernt-matthias
Copy link
Contributor

if the JobWrapper.fail method is called directly exit code (etc)
are lost.

fixes #14206

not sure why this error popped up now, probably a change in
tool verification ..?

For fixing the IUC CI job we probably need to backport this .. is 22.01 sufficient?

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these contributions under Galaxy's current license.
  • I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to [email protected].

if the JobWrapper.fail method is called directly exit code (etc)
are lost.

fixes galaxyproject#14206

not sure why this error popped up now, probably a change in
tool verification ..?
@github-actions github-actions bot added this to the 22.09 milestone Jun 28, 2022
@mvdbeek
Copy link
Member

mvdbeek commented Jun 28, 2022

not sure why this error popped up now, probably a change in tool verification ..?

planemo started using outputs_to_working_directory with your PR (galaxyproject/planemo#1199), and we only recently cut a new release. Maybe backport to 21.09, then we (almost) cover the whole year of supported releases.

Any chance you could add test tool for this and add it to https://github.com/galaxyproject/galaxy/blob/dev/test/integration/test_job_outputs_to_working_directory.py#L20 ?

@bernt-matthias
Copy link
Contributor Author

Will add a test

should be covered by a small change to strict_shell_profile.xml
(added analogous changes to strict_shell_*.xml)
@bernt-matthias
Copy link
Contributor Author

bernt-matthias commented Jun 28, 2022

If the test passes I will revert 6803572 .. to see if it works as desired...

@bernt-matthias bernt-matthias marked this pull request as draft June 28, 2022 12:48
@bernt-matthias
Copy link
Contributor Author

@mvdbeek any idea why the test tool isn't failing after reverting the fix? Is the tool / the test environment missing something?

@bernt-matthias
Copy link
Contributor Author

Galaxy seems to create the output files (empty) in the jwd before the job .. so it appears that the failing dada2 tool calls an rm on the output file which provokes this error.

Will adapt the test tool accordingly (or better create a new one).

I think I will create a new PR targeting 21.09.

@bernt-matthias
Copy link
Contributor Author

superseded by #14235

@bernt-matthias bernt-matthias deleted the topic/fail-ec branch July 1, 2022 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tool test that is expected to fail does not get the exit code
2 participants