Stream Local Mode Output on the same Process. #203
Conversation
For Serving containers the container would start as a child process using Popen, the stdout/stderr are sent to the terminal as usual, however on Jupyter notebooks this does not happen because Jupyter does not show the output of child processes. This instroduces a Thread that prints the output from a subprocess PIPE. By doing this, the output comes from the main process and will be shown in Jupyter notebooks.
Codecov Report
@@ Coverage Diff @@
## master #203 +/- ##
==========================================
- Coverage 91.03% 90.79% -0.24%
==========================================
Files 42 42
Lines 2699 2717 +18
==========================================
+ Hits 2457 2467 +10
- Misses 242 250 +8
Continue to review full report at Codecov.
|
iquintero
force-pushed
the
lm_log_improvement
branch
from
654ee0e to
9d92345
Compare
| # _stream_output() doesn't have the command line. We will handle the exception | ||
| # which contains the exit code and append the command line to it. | ||
| msg = "Failed to run: %s, %s" % (compose_command, e.message) | ||
| raise RuntimeError(msg) |
There was a problem hiding this comment.
can you also include e as well
| def __init__(self, command, startup_delay=5): | ||
| class _HostingContainer(Thread): | ||
| def __init__(self, command): | ||
| Thread.__init__(self) |
There was a problem hiding this comment.
Okay if Thread isn't a new style class.
| def up(self): | ||
| self.process = Popen(self.command) | ||
| sleep(self.startup_delay) | ||
| def run(self): |
There was a problem hiding this comment.
This is identical to the code in train, just with a different command. Suggest refactoring into a utility function.
| process = subprocess.Popen(compose_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) | ||
|
|
||
| try: | ||
| _stream_output(process) |
There was a problem hiding this comment.
Do you need this.
Doesn't something like this work:
>>> subprocess.check_call(["ls", "asdf"], stderr=sys.stdout.fileno())
ls: asdf: No such file or directory
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/Cellar/python/2.7.14/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 186, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['ls', 'asdf']' returned non-zero exit status 1I think the interface is a bit different in python 3, but we can work through this.
There was a problem hiding this comment.
Talked IRL, this won't work in Jupyter because Jupyter replaces the stderr object.
Fixed: Seq2seq misnamed sagemaker client
3 participants
For Serving containers the container would start as a child process
using Popen, the stdout/stderr are sent to the terminal as usual,
however on Jupyter notebooks this does not happen because Jupyter does
not show the output of child processes.
This instroduces a Thread that prints the output from a subprocess PIPE.
By doing this, the output comes from the main process and will be shown
in Jupyter notebooks.
Issue #, if available:
Description of changes:
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.