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

add subprocess_util module to replace sh #40

Merged
merged 4 commits into from
Apr 5, 2023

Conversation

gotmax23
Copy link
Collaborator

@gotmax23 gotmax23 commented Mar 21, 2023

This adds code to run subprocesses asynchronously and integrates with the logger. stdout and stderr are logged in real time and always captured. We return a CompletedProcess object like regular subprocess does.

Relates: #34

@gotmax23
Copy link
Collaborator Author

gotmax23 commented Mar 21, 2023

I want to add unit tests but I don't think caplog works with twiggy...

@gotmax23 gotmax23 force-pushed the subprocess_util branch 5 times, most recently from 4e005b3 to 4d1f1fc Compare March 25, 2023 16:43
@gotmax23
Copy link
Collaborator Author

I was very perturbed as to why the tests were failing in CI but locally, but I guess for i in {1..15}; do is a bashism. Fedora sets /bin/sh to bash while Debian and Ubuntu set it to dash.

This adds code to run subprocesses asynchronously and integrates with
the logger. stdout and stderr are logged in real time and always
captured. We return a CompletedProcess object like regular subprocess
does.

Relates: ansible-community#34
Both Logger classes have `.debug()`, `.info()`, etc. methods, so let's
explicitly declare support for both.
@gotmax23 gotmax23 changed the title [wip] add subprocess_util module to replace sh add subprocess_util module to replace sh Mar 25, 2023
@gotmax23 gotmax23 requested a review from felixfontein March 25, 2023 17:51
@felixfontein felixfontein merged commit 0388c0f into ansible-community:main Apr 5, 2023
@felixfontein
Copy link
Collaborator

@gotmax23 thanks for adding this!

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