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

making improvements to the consistency and functionality of the shell code #31

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

aaronbog
Copy link
Collaborator

@aaronbog aaronbog commented May 22, 2024

changes

shell/utils.py:

shell/shell.py:

  1. rewritten shell_out function
  • using true to determine the value of the success return value
  • refactor such that all commands can use the same Popen structure for consistency
  • output_is_log=True now takes into account the timeout variable (this is the use of the multiprocessing package)
  • output_is_log=True now takes into account the ignore_ret_codes
  • made allowed exit codes add their stderr to the output instead of overwriting it
  • added split_arguments=False as an argument so pipe_shell_out can be simplified
  1. rewritten pipe_shell_out function
  • rewrote the implementation using shell_out and the new split_arguments argument

remarks:

  • the usage of true to determine the value of the success might not be that useful but it would be an early warning that it is a non possix compliant operating system

suggestions:

  • pipe_shell_out could and should be implemented differently by linking the stdin and stdout of two Popen together since using shell=true is discouraged
  • rename std_input to input since the way it is used is as a text string and not as an inputstream

…g return value to a queue to eliminate errors.
@apaolillo apaolillo self-assigned this Jun 17, 2024
@apaolillo apaolillo self-requested a review June 17, 2024 15:26
@apaolillo apaolillo added the shellout Changes to the internal of executing something on the shell; label Jun 17, 2024
@apaolillo apaolillo marked this pull request as draft August 10, 2024 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
shellout Changes to the internal of executing something on the shell;
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants