Pipe improvement #198
Merged
Pipe improvement #198
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ommand Now a real pipe is created to a subprocess. This has many advantages and should "just work" like intended with all commands. One downside is to work properly on Python 2.7, it requires the subprocess32 module which is the subprocess module from Python 3.2 backported to Python 2.7. Another downside, is that unit testing the feature is now more difficult. This still needs to be tested for compatibility across all OSes and supported versions of Python. The user needs to be careful if designing multi-threaded cmd2 applications that do command processing in other threads and those threads can make calls to self.stdout.write to put in a try/except to catch Broken Pipe errors which can occur for long running commands if the user closes the shell subprocess before the command is finished.
Open them in text mode in Python 3 so self.stdout.write() expects normal Python 3 (unicode) strings. Open them in binary mode in Python 2 so self.stdout.write() expects normal Python 2 (byte) strings. Also fixed a unit test mocking issue on Python 2 to account for the fact that Python 2.7 requires the subprocess32 module instead of subprocess.
Codecov Report
@@ Coverage Diff @@
## master #198 +/- ##
==========================================
- Coverage 97.1% 96.79% -0.31%
==========================================
Files 1 1
Lines 1139 1155 +16
==========================================
+ Hits 1106 1118 +12
- Misses 33 37 +4
Continue to review full report at Codecov.
|
…ut(). Modified implementation of poutput() to accept an optional "end" argument which can change the ending to something other than a newline or simply suppress adding a newline. Also added a try/except to catch BrokenPipeError exceptions which can occupoutputr if the subprocess output is being piped to closes before the command piping output to it is finished. Updated install docs to note that when using Python 2.7, the subprocess32 module should also be installed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous implementation for piping command output to a shell command utilized a temporary file and only once the command was completely done did it "pipe" the command output to the shell command by using the temporary file as the commands input. This approach had many disadvantages, including:
Now a real pipe is created to a subprocess. This approach should "just work like intended" with all commands and has many advantages, including:
One downside is to work properly on Python 2.7, it requires the subprocess32 module which is the subprocess module from Python 3.2 backported to Python 2.7. Another downside, is that unit testing the feature is now more difficult.
A more subtle downside is that users now need to be careful when designing multi-threaded cmd2 applications that do command processing in other threads where those threads can make calls to self.stdout.write. If the end user does something to manually close the piped shell command before the cmd2 command is finished executing, then a BrokenPipe exception can occur. This is easily fixed by putting a try/except to catch Broken Pipe errors in one strategic location in your code. You just need to be aware of this.
This closes #197