Fix local mode error message #272
Conversation
Codecov Report
@@ Coverage Diff @@
## master #272 +/- ##
==========================================
+ Coverage 92.34% 92.43% +0.09%
==========================================
Files 49 49
Lines 3279 3279
==========================================
+ Hits 3028 3031 +3
+ Misses 251 248 -3
Continue to review full report at Codecov.
|
| def test_train_error(_download_folder, _cleanup, popen, _stream_output, LocalSession, tmpdir, sagemaker_session): | ||
| directories = [str(tmpdir.mkdir('container-root')), str(tmpdir.mkdir('data'))] | ||
|
|
||
| with patch('sagemaker.local.image._SageMakerContainer._create_tmp_folder', side_effect=directories): |
There was a problem hiding this comment.
why not put it as a patch as well?
There was a problem hiding this comment.
since it relies on directories and I doubt putting that variable assignment directly in the patch annotation would fit in one line
| image = 'my-image' | ||
| sagemaker_container = _SageMakerContainer('local', instance_count, image, sagemaker_session=sagemaker_session) | ||
|
|
||
| with pytest.raises(RuntimeError) as e: |
There was a problem hiding this comment.
I dont think this should be one single test. I would split it to be a serving_container_handles_failure or something.
There was a problem hiding this comment.
I'm not entirely sure what you mean here. _stream_output is patched to return a RuntimeError, so all this is testing is that the eventual RuntimeError returned by train() contains the error message from the first RuntimeError
Issue #, if available:
#261
Description of changes:
In the local mode image code, we were incorrectly calling
e.message, which doesn't work in Python. This change is to remedy that so RuntimeErrors get handled correctly.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.