aws / aws/sagemaker-pytorch-inference-toolkit

Prepend `code_dir` to `sys.path` rather than `append`

Open
#137 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.