Skip to content

Fix: Don't load default model in MME mode #130

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.

Sign up for GitHub

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

Merged
merged 16 commits into from
Nov 7, 2022
Merged

Fix: Don't load default model in MME mode #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
69eeea9
Fix: Don't load default model in MME mode
nikhil-sk Oct 31, 2022
8b210dd
Remove whitespace from blank line
nikhil-sk Oct 31, 2022
8514322
Fix flake8 issue in other file
nikhil-sk Oct 31, 2022
70b1278
Remove --model specification from MME test
nikhil-sk Oct 31, 2022
b67f7fa
Trigger build
nikhil-sk Oct 31, 2022
17094ed
Upgrade sagemaker and torch package
nikhil-sk Oct 31, 2022
dbfab99
Unpin sagemaker in tox
nikhil-sk Oct 31, 2022
587efc9
Pin protobuf version
nikhil-sk Oct 31, 2022
260288f
Update protobuf version for python3.6
nikhil-sk Oct 31, 2022
bb2945f
Trigger build
nikhil-sk Oct 31, 2022
ce931fd
Revert dependency changes, keep only test dep changes
nikhil-sk Nov 1, 2022
604e65b
Trigger build
Nov 1, 2022
ba3c453
Revert version change when setting env variables
nikhil-sk Nov 3, 2022
237a0e1
Merge branch 'mme_fix' of https://github.com/nskool/sagemaker-pytorch…
nikhil-sk Nov 3, 2022
b024a42
Trigger build
nikhil-sk Nov 4, 2022
e1a9745
Trigger build
nikhil-sk Nov 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/sagemaker_pytorch_serving_container/torchserve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,13 @@ def start_torchserve(handler_service=DEFAULT_HANDLER_SERVICE):
TS_CONFIG_FILE,
"--log-config",
DEFAULT_TS_LOG_FILE,
"--models",
DEFAULT_TS_MODEL_NAME + "=" + environment.model_dir
]

default_model_path_args = ["--models", DEFAULT_TS_MODEL_NAME + "=" + environment.model_dir]

if not ENABLE_MULTI_MODEL:
ts_torchserve_cmd += default_model_path_args

print(ts_torchserve_cmd)

logger.info(ts_torchserve_cmd)
Expand Down
2 changes: 0 additions & 2 deletions test/unit/test_model_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,6 @@ def test_start_torchserve_default_service_handler_multi_model(
torchserve.TS_CONFIG_FILE,
"--log-config",
torchserve.DEFAULT_TS_LOG_FILE,
"--models",
"model=/opt/ml/model"
]

subprocess_popen.assert_called_once_with(ts_model_server_cmd)
Expand Down
2 changes: 1 addition & 1 deletion test/utils/file_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
def make_tarfile(script, model, output_path, filename="model.tar.gz", script_path=None):
output_filename = os.path.join(output_path, filename)
with tarfile.open(output_filename, "w:gz") as tar:
if(script_path):
if (script_path):
tar.add(script, arcname=os.path.join(script_path, os.path.basename(script)))
else:
tar.add(script, arcname=os.path.basename(script))
Expand Down
1 change: 1 addition & 0 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ deps =
six
future
pyyaml
protobuf == 3.19.6
Copy link
Contributor Author

@nikhil-sk nikhil-sk Oct 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is currently required, as otherwise, SageMaker imports fail with error on py37 only:

    import sagemaker.amazon.common
.tox/py37/lib/python3.7/site-packages/sagemaker/amazon/common.py:23: in <module>
  from sagemaker.amazon.record_pb2 import Record
.tox/py37/lib/python3.7/site-packages/sagemaker/amazon/record_pb2.py:52: in <module>
    file=DESCRIPTOR,
.tox/py37/lib/python3.7/site-packages/google/protobuf/descriptor.py:560: in __new__
    _message.Message._CheckCalledFromGeneratedFile()
E   TypeError: Descriptors cannot not be created directly.
E   If this call came from a _pb2.py file, your generated code is out of date and must be regenerated with protoc >= 3.19.0.
E   If you cannot immediately regenerate your protos, some other possible workarounds are:
E    1. Downgrade the protobuf package to 3.20.x or lower.
E    2. Set PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python (but this will use pure-Python parsing and will be much slower).
E
E   More information: https://developers.google.com/protocol-buffers/docs/news/2022-05-06#python-updates

Upgrading SageMaker version did not resolve the issue, so currently we need to pin the version, and consider a complete upgrade of dependencies in a separate PR.


[testenv:flake8]
basepython = python3
Expand Down