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

Bugfix subprocess error handling #774

Closed
wants to merge 4 commits into from

Conversation

PhilippeMoussalli
Copy link
Contributor

Setting capture_output=True captures both the default outputstdout and the error output stderr in the output variable which means that we're not streaming anymore the standard output of docker-compose up.

This PR fixes this by only capturing the error stderr in the output, the standard output is streamed

Before:

Traceback (most recent call last):
  File "/home/philippe/Scripts/express/a/launch.py", line 14, in <module>
    raise Exception(msg)
Exception: Command failed with error: ' Container a-web-1  Created
a-web-1  | Traceback (most recent call last):
a-web-1  |   File "app.py", line 4, in <module>
a-web-1  |     print(1 / 0)
a-web-1  | ZeroDivisionError: division by zero
Aborting on container exit...
 Container a-web-1  Stopping
 Container a-web-1  Stopped

After:

Attaching to a-web-1
a-web-1  | This is a simple Python script.  #Standard output of print statement contained in the script 
a-web-1 exited with code 1
Traceback (most recent call last):
  File "/home/philippe/Scripts/express/a/launch.py", line 14, in <module>
    raise Exception(msg)
Exception: Command failed with error: ' Container a-web-1  Created
a-web-1  | Traceback (most recent call last):
a-web-1  |   File "app.py", line 4, in <module>
a-web-1  |     print(1 / 0)
a-web-1  | ZeroDivisionError: division by zero
Aborting on container exit...
 Container a-web-1  Stopping
 Container a-web-1  Stopped

@RobbeSneyders
Copy link
Member

Is it an issue that stderr is no longer streamed? I think there's ways around it, but they require using Popen directly I believe.

@PhilippeMoussalli
Copy link
Contributor Author

PhilippeMoussalli commented Jan 11, 2024

Is it an issue that stderr is no longer streamed? I think there's ways around it, but they require using Popen directly I believe.

Not streaming stderr is fine because we will raise it anyways when we catch the error. I think the issue is mainly with streaming stdout since we don't have that logs that are appearing anymore. Setting capture_output=True seem to block streaming for both output types which is why now we only capture stderr

Screenshot from 2024-01-11 16-48-35

@PhilippeMoussalli
Copy link
Contributor Author

Closed in favor of #775

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