aws / aws/sagemaker-pytorch-inference-toolkit
Prepend `code_dir` to `sys.path` rather than `append`
- Dominant language
- Python
- Stars
- 143
- Forks
- 73
- PR merge metrics
- No merged PRs in 30d
Description
**Describe the bug**
The bug is that the semantics of running python scripts prioritize the directory containing the script by prepending to `sys.path`. To have similar semantics, we should prepend `code_dir` to `sys.path` rather than append here:
https://github.com/aws/sagemaker-pytorch-inference-toolkit/blob/6610a410c0cf40bcf15267abe722d20d50e77bcf/src/sagemaker_pytorch_serving_container/handler_service.py#L46-L49
Here's a quick example showing the prepend semantics when running a script from the command line.
1. First, put this in a file './code/myscript.py' in a local shell environment:
``` python
import sys
print(sys.path)
```
2. Then run it:
``` bash
$ pwd
/home/ubuntu
$ python3 ./code/main.py
['/home/ubuntu/code', '/usr/lib/python38.zip', '/usr/lib/python3.8', '/usr/lib/python3.8/lib-dynload', '/usr/local/lib/python3.8/dist-packages', '/usr/lib/python3/dist-packages']
```
The current appending behavior would cause an issue for a customer put a filename in `code_dir` that clashed with an installed package. If the customer ran their inference script locally, it would load their file due to prepend semantics, but when deploying to MME with this toolkit's handler, it would prioritize the installed package instead.
The single-model endpoint case is already prepended: https://github.com/aws/sagemaker-pytorch-inference-toolkit/blob/fb65d8a982944fe82449e964bc8021a9f6e1d869/src/sagemaker_pytorch_serving_container/torchserve.py#L112-L120
Other sagemaker inference toolkits already prepend, as well. See how `sagemaker-huggingface-inference-toolkit` handles this (https://github.com/aws/sagemaker-huggingface-inference-toolkit/blob/2f1fae5cbb3b68299e73cc591c0a912b7cccee29/src/sagemaker_huggingface_inference_toolkit/handler_service.py#L72-L73), as well as how `sagemaker-inference-toolkit` and `sagemaker-mxnet-inference-toolkit` *try* to handle this (though they have their own bug in this part of the code--see https://github.com/aws/sagemaker-mxnet-inference-toolkit/issues/135).
Contributor guide
Research direction
Start in src/sagemaker_pytorch_serving_container/handler_service.py at the referenced sys.path handling around lines 46–49. Compare it with the prepended path behavior in src/sagemaker_pytorch_serving_container/torchserve.py and the linked Hugging Face toolkit implementation. Done means code_dir has the same precedence as when a Python script is run locally, including over similarly named installed packages.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, machine-learning
- Issue type
- Bug
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100