Skip to content
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.

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

Prepend code_dir to sys.path rather than append #137

Open
davidthomas426 opened this issue Feb 17, 2023 · 0 comments
Open

Prepend code_dir to sys.path rather than append #137

davidthomas426 opened this issue Feb 17, 2023 · 0 comments

Comments

@davidthomas426
Copy link

davidthomas426 commented Feb 17, 2023

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:

if (not self._initialized) and ENABLE_MULTI_MODEL:
code_dir = os.path.join(context.system_properties.get("model_dir"), 'code')
sys.path.append(code_dir)
self._initialized = True

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:
import sys
print(sys.path)
  1. Then run it:
$ 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:

def _set_python_path():
# Torchserve handles code execution by appending the export path, provided
# to the model archiver, to the PYTHONPATH env var.
# The code_dir has to be added to the PYTHONPATH otherwise the
# user provided module can not be imported properly.
if PYTHON_PATH_ENV in os.environ:
os.environ[PYTHON_PATH_ENV] = "{}:{}".format(environment.code_dir, os.environ[PYTHON_PATH_ENV])
else:
os.environ[PYTHON_PATH_ENV] = environment.code_dir

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 aws/sagemaker-mxnet-inference-toolkit#135).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant