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

Script execution does not relay exit code #71

Closed
cwichel opened this issue Jun 3, 2022 · 9 comments
Closed

Script execution does not relay exit code #71

cwichel opened this issue Jun 3, 2022 · 9 comments
Labels
bug Something isn't working

Comments

@cwichel
Copy link

cwichel commented Jun 3, 2022

Environment

  • OS: Windows 10
  • Poetry: 1.2.0b1
  • POE: 0.13.1

Issue

If you have a script that returns an error as system exit code, for example:

include sys

def test_script() -> None:
    sys.exit(1) 

In a file called sample.py in your repository root, and configured as follows on the pyproject.toml file:

[tool.poe.tasks]
    [tool.poe.tasks.test-script]
    help    = "Check if the returnvalue is being relayed"
    script  = "sample:test_script"

You'll see (windows):

> poe test-script
Poe => test-script
> echo %errorlevel%
0

Instead of:

> python .c "import sample as s; s.test_script()"
> echo %errorlevel%
1

Is this intended? Is there a way to relay the returncode from the failed script out of poe?

@nat-n
Copy link
Owner

nat-n commented Jun 4, 2022

Hi @cwichel,

Thanks for the feedback. Something is off here. Poe is meant to propagate the exit code of tasks. When I try an example like your on macOS it works as you would expect.

I would like to determine what's going on here and get to a fix if possible. Unfortunately I don't have easy access to a windows dev environment to debug with, and my understanding of windows specifics are quite limited, so I'd appreciate if you (or another windows user out there?) could help with investigating this issue.

For context the way it's meant to work is that:

  1. The script task dynamically composes a python script that loads and calls the target function like so:
    cmd = (
    "python",
    "-c",
    "import sys; "
    "from os import environ; "
    "from importlib import import_module; "
    f"sys.argv = {argv!r}; sys.path.append('src');"
    f"\n{self.format_args_class(self.named_args)}"
    f"import_module('{target_module}').{function_call}",
    )
  2. the python cmd is executed as a subprocess, and it's return code is passed all the way back up the stack to the main function here:
    result = app(cli_args=sys.argv[1:])
    if result:
    raise SystemExit(result)

Can you think of any reason the that logic would not have the intended result on windows?

@cwichel
Copy link
Author

cwichel commented Jun 4, 2022

Hey @nat-n

It seems that the issue is not coming from poe... Doing some testing shows that, at least on my machine, running the command with poetry is returning the incorrect error code. I've included a print of the command on L153 here:

# TODO: exclude the subprocess from coverage more gracefully
_stop_coverage()
proc = Popen(cmd, **popen_kwargs)

And it returns:

> poe test-script
('C:\\Tools\\Miniconda3\\Scripts\\poetry.EXE', 'run', 'python', '-c', "import sys; from os import environ; from importlib import import_module; sys.argv = ['test-script']; sys.path.append('src');\nimport_module('sample.tests').test_script()")
> echo %errorlevel%
0

If I try to execute the line directly using python (no poetry run):

> python -c "import sys; from os import environ; from importlib import import_module; sys.argv = ['test-script']; sys.path.append('src'); import_module('sample.tests').test_script()
> echo %errorlevel%
1

But using poetry...

>  poetry run python -c "import sys; from os import environ; from importlib import import_module; sys.argv = ['test-script']; sys.path.append('src'); import_module('sample.tests').test_script()
> echo %errorlevel%
0

So it seems that the issue needs to go there 😄

As a separate topic... why did you include a line jump on L56 in 1. (just to know)?

@nat-n
Copy link
Owner

nat-n commented Jun 4, 2022

Thanks for investigating @cwichel

That's a pity. I guess the solution would be either to try get a fix into poetry (which could take a while with poetry's release rate), or to stop using poetry run which might be a good idea but may be riskier in terms of covering all edge cases. I'll have a think about it. Let me know if you open an issue on the poetry repo.

The new line on L56 is necessary in case of passing CLI arguments to the python function. When this happens format_args_class returns a class, and of course the class keyword returns a new line.

For now I'll leave this open as a bug.

@cwichel
Copy link
Author

cwichel commented Jun 10, 2022

@nat-n hello again!
It seems that the issue is not related to poetry but the compatibility of POE on the newer (beta) versions...

First test: Poetry v1.1.13

  1. Prepare environment
    > conda create -n test_1 python=3.7.11 -y & activate test_1
    > pip install poethepoet poetry==1.1.13 
    
  2. Run the "test":
    1. Dry run (no execution through POE):
      > poetry run python -c "import sys; sys.exit(1)"
      > echo %errorlevel%
      1
      
    2. Run through POE:
      > poe test-script
      Poe => test-script
      > echo %errorlevel%
      1
      
    So, as you can see in this configuration everything is working as expected...

First test: Poetry v1.2.0b2

  1. Prepare environment
    > conda create -n test_2 python=3.7.11 -y & activate test_2
    > pip install poethepoet poetry==1.2.0b2 
    
  2. Run the "test":
    1. Dry run (no execution through POE):
      > poetry run python -c "import sys; sys.exit(1)"
      > echo %errorlevel%
      0
      
      Seems that poetry stopped working...
    2. Run through POE:
      > poe test-script
      Poe => test-script
      > echo %errorlevel%
      0
      
      And thus, the same happened with POE...
    3. But if we uninstall POE and try again...
      > pip uninstall poethepoet -y
      > poetry run python -c "import sys; sys.exit(1)"
      > echo %errorlevel%
      1
      
      Poetry starts working!

I imagine that this must be related to the new capabilities added to poetry and the adjustment to their backend to achieve them (for example, the new plugin system). Also, I assume that this is not a priority for POE until the v1.2 of poetry gets actually released...

@nat-n
Copy link
Owner

nat-n commented Jun 10, 2022

Curious indeed. Only thing I can think of is that somehow poe being picked up as a plugin for poetry somehow changes the way poetry runs stuff in general?!

Thanks for investigating. Lets keep an eye on this.

@nat-n
Copy link
Owner

nat-n commented Jul 16, 2022

Hi @cwichel, could you be so kind as to confirm whether this is still an issue with poetry >= 1.2.0b3 ?

@cwichel
Copy link
Author

cwichel commented Jul 21, 2022

Hey @nat-n! I've performed the same steps shown in my comment from June 10th, but changed the poetry version to 1.2.0b3 as requested. The results are the same: If installed with poe the error code returned is 0 independently of the value provided to sys.exit.

@cwichel
Copy link
Author

cwichel commented Oct 14, 2022

@nat-n hey! I've performed the same test steps today using:

  • poetry: 1.2.2
  • poe: 0.16.4

And it's working now!

> poetry run python -c "import sys; sys.exit(13)"
> echo %errorlevel%
13 

@nat-n
Copy link
Owner

nat-n commented Oct 15, 2022

@cwichel awesome, thanks for reporting back! case closed then.

@nat-n nat-n closed this as completed Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants