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

conda run is not preserving exit code #9599

Closed
impredicative opened this issue Jan 14, 2020 · 11 comments
Closed

conda run is not preserving exit code #9599

impredicative opened this issue Jan 14, 2020 · 11 comments
Labels
locked [bot] locked due to inactivity

Comments

@impredicative
Copy link

impredicative commented Jan 14, 2020

Current Behavior

conda run used to return the correct exit status of the target process, but it doesn't anymore. This was apparently working correctly in conda 4.8.0 but it seems broken since version 4.8.1. conda run now erroneously always return the exit status of 0. This is a serious problem as jobs fail silently now.

Steps to Reproduce

$ conda -V
conda 4.8.1
$ conda run -n myenv /usr/bin/true; echo $?
0
$ conda run -n myenv /usr/bin/false; echo $?
ERROR conda.cli.main_run:execute(30): Subprocess for 'conda run ['/usr/bin/false']' command failed.  (See above for error)
0

Expected Behavior

conda run should preserve the exit status of the target process, whether the target is python or otherwise. It worked correctly with the previous version:

$ conda -V
conda 4.8.0
$ conda run -n base /usr/bin/true; echo $?
0
$ conda run -n base /usr/bin/false; echo $?
ERROR conda.cli.main_run:execute(39): Subprocess for 'conda run ['/usr/bin/false']' command failed.  Stderr was:

1

Environment Information

This issue really is independent of the environment as it happens both in various dockerfiles and on MacOS too.


@msarahan Please fix this bug considering I see a commit from 25 days that may have introduced it! Adding unit tests would also be wise.

@mingwandroid
Copy link
Contributor

mingwandroid commented Jan 15, 2020

Our team have many things to work on. PRs that try to fix this would be welcome if you are interested. Conda run is still not ready for prime time usage unfortunately.

@mingwandroid
Copy link
Contributor

We are not churning out new conda features.

@mlline00
Copy link
Contributor

mlline00 commented Feb 2, 2020

It appears commit c7d5ada introduced the error.

Returning a namedtuple (Response) instead of an int breaks this logic

conda/conda/cli/main.py

Lines 84 to 86 in e9a5056

exit_code = do_call(args, p)
if isinstance(exit_code, int):
return exit_code

I can create a pull request to fix. Thoughts on reverting the return value to an int vs expanding the logic at the call site in main.py? I can't find rationale for changing the return value.

@impredicative
Copy link
Author

impredicative commented Feb 2, 2020

The scarier point as I see it is that conda run being experimental is an excuse for the conda team to break it willy-nilly and to not bother to fix it, to eschew responsibility. Even if this issue is fixed, I cannot trust using conda run while it stays experimental.

If I recall correctly, conda init is also experimental, and I guess I can't use that either in my production code.

I now use a simple PATH based activation approach instead in my Dockerfile.

@impredicative
Copy link
Author

impredicative commented Feb 3, 2020

FWIW, here is a high-level unit test to test the functionality after a fix is implemented:

import subprocess
import unittest


class TestExitCodes(unittest.TestCase):
    # Note: conda=4.8.1 (but not conda=4.8.0) always incorrectly returns exit code 0.
    def test_exit_codes(self):
        for expected_exit_code in range(3):
            with self.subTest(exit_code=expected_exit_code):
                actual_exit_code, _output = subprocess.getstatusoutput(f"conda run exit {expected_exit_code}")
                self.assertEqual(expected_exit_code, actual_exit_code)


if __name__ == "__main__":
    unittest.main()

If using this test with a failing conda, e.g. 4.8.1 or 4.8.2, consider adding the decorator @unittest.expectedFailure to the test.

@esc
Copy link
Contributor

esc commented Feb 6, 2020

I'd like to see a fix for this also.

esc added a commit to esc/texasbbq that referenced this issue Feb 6, 2020
As of conda 0.48.1 `conda run -n` doesn't exit with the code that the
command it ran exited with. This causes all the Numba integration tests
to silently be successful. A PR is in flight to fix this already, but we
hotfix it for now to get the tests up and running again.

Issue:

conda/conda#9599

PR:

conda/conda#9665
@esc
Copy link
Contributor

esc commented Apr 12, 2020

I believe this is a duplicate of #8385 perhaps?

@jjhelmus
Copy link
Contributor

@impredicative I have removed you latest comment from this issue. It is not constructive and the tone is not welcome here. If you continue to make comments along these lines your account will be banned.

@conda conda deleted a comment from impredicative Apr 13, 2020
@esc
Copy link
Contributor

esc commented Apr 15, 2020

Could someone perhaps re-open this issue. This isn't a duplicate of #8385 after all. But a regression of that issue. There also already seems to be a PR to fix both at #9665 .

chenghlee added a commit that referenced this issue Jul 22, 2020
set errorlevel when 'conda run' exits (Resolves #9599)
@impredicative
Copy link
Author

Finally this is fixed and released in v4.8.4. Thank you @chenghlee !

conda run however continues to remain experimental and therefore unfit for production use.

@github-actions
Copy link

Hi there, thank you for your contribution to Conda!

This issue has been automatically locked since it has not had recent activity after it was closed.

Please open a new issue if needed.

@github-actions github-actions bot added the locked [bot] locked due to inactivity label Aug 20, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked [bot] locked due to inactivity
Projects
None yet
Development

No branches or pull requests

5 participants