aws / aws/sagemaker-training-toolkit

Adding sys.path to PYTHONPATH breaks virtual environments

Open
#186 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
530
Forks
140
Avg merge
1h 12m
Merged PRs (30d)
2

Description

**Describe the bug**

In [src/sagemaker_training/environment.py:1219](https://github.com/aws/sagemaker-training-toolkit/blob/37926a2cc522175fa1458a647965f47e4c8043d3/src/sagemaker_training/environment.py#L1219C8-L1219C8), the values in `sys.path` (of the environment in which sagemaker-training-toolkit is executed) are added to the environment variable `$PYTHONPATH`.

This breaks virtual environments, because the result is that `$PYTHONPATH` ends up pointing to libraries in the `base` environment. So when we run code in a virtual conda environment, that environments' `sys.path` ends up looking at the `$PYTHONPATH` contents, which were set to the base environment's `sys.path`. In effect, this overwrites a virtual environments `sys.path` with the base environment's `sys.path` due to the `sys.path` search order [link](https://docs.python.org/3/library/sys.html#sys.path).

The end result is that the base environment libraries are given precedence over the virtual environment libraries.

We have a solution for now, which is to simply `unset PYTHONPATH` before we do anything with virtual environments.

However, I'm filing a bug report because it would be preferable for virtual environments to work out-of-the-box, and because this was a particularly difficult problem to find as it can present in so many different ways.

_(For us, this issue was discovered because it led to a conda virtual environment with python 3.9 crashing because it attempted to use python 3.10 libraries in the base environment. Note that the error below is not important to understanding the issue, as there are many ways this issue can present itself - I'm only adding it to help others find this issue.)_

```
Fatal Python error: init_sys_streams: can't initialize sys standard streams
Traceback (most recent call last):
File "/opt/conda/lib/python3.10/io.py", line 54, in
ImportError: cannot import name 'text_encoding' from 'io' (unknown location)
```

**To reproduce**

See the minimal example below to reproduce. In this minimal example I install two different versions of `isort`, namely `5.11.4` in `base` conda environment and `5.12.0` in `dev` conda environment. The output of the example is as follows, which shows the problem in action:

```bash
***** python location and version *****
(BASE): conda run --no-capture-output --live-stream -n base which python && python --version
/opt/conda/bin/python
Python 3.10.12
(DEV): conda run --no-capture-output --live-stream -n dev which python && python --version
/opt/conda/envs/dev/bin/python
Python 3.10.12

***** isort version seen by conda *****
(BASE): conda run --no-capture-output --live-stream -n base conda list isort
# packages in environment at /opt/conda:
isort 5.11.4 pyhd8ed1ab_1 conda-forge
(DEV): conda run --no-capture-output --live-stream -n dev conda list isort
# packages in environment at /opt/conda/envs/dev:
isort 5.12.0 pyhd8ed1ab_1 conda-forge

***** isort version inside python *****
(BASE): conda run --no-capture-output --live-stream -n base python -c 'import isort; print(isort.__version__)'
5.11.4
(DEV): conda run --no-capture-output --live-stream -n dev python -c 'import isort; print(isort.__version__)'
5.11.4

***** isort version inside python - after unset PYTHONPATH*****
(BASE): unset PYTHONPATH && conda run --no-capture-output --live-stream -n base python -c 'import isort; print(isort.__version__)'
5.11.4
(DEV): unset PYTHONPATH && conda run --no-capture-output --live-stream -n dev python -c 'import isort; print(isort.__version__)'
5.12.0
```

To reproduce the above example, use the files `problem.Dockerfile`, `launcher.py` and `wrong_version_demo.py` as defined below (inserting appropriate values for your aws account + region where `` is written), and then locally do:

```bash
conda create -n problem
conda activate problem
mamba install sagemaker-python-sdk=2.166.0

docker build --file problem.Dockerfile --target base -t .
python launcher.py
```

```problem.Dockerfile```
```Dockerfile
# syntax=docker/dockerfile:1
FROM condaforge/mambaforge:23.1.0-2 as base

ARG DEBIAN_FRONTEND=noninteractive

RUN apt-get update -yqq
RUN apt-get install -yqq build-essential
RUN apt-get install -yqq python3-dev
RUN apt-get clean
RUN rm -rf /var/lib/apt/lists/* /var/lib/{apt,dpkg,cache,log}

RUN conda config --set always_yes true \
&& conda config --set unsatisfiable_hints true \
&& conda config --prepend channels defaults \
&& conda config --prepend channels conda-forge

RUN pip3 install sagemaker-training==4.6.1

RUN conda install -n base isort=5.11.4
RUN conda create -n dev python=3.10 isort=5.12.0

WORKDIR /work
```

```launcher.py```
```python
from pathlib import Path
import sagemaker
from sagemaker.estimator import Estimator

def main():
sagemaker_session = sagemaker.LocalSession()
current_file_path = Path(__file__).resolve().absolute()
local_script_path = current_file_path.parent / "wrong_version_demo.py"

image_uri=""
role=""
subnets=[""],
security_group_ids=[""],

# Launch the training job
estimator = Estimator(
environment={"SAGEMAKER_PROGRAM": "wrong_version_demo.py"},
image_uri=image_uri,
role=role,
subnets=subnets,
security_group_ids=security_group_ids,
instance_count=1,
instance_type="local",
max_run=1*60*60,
sagemaker_session=sagemaker_session,
entry_point=local_script_path
)
estimator.fit(inputs=None, job_name="problem-job", wait=True)

if __name__ == "__main__":
main()
```

```wrong_version_demo.py```
```python
import os
import subprocess
import sys

def demo():

print("="*100)

print(f"{'*'*5} python location and version {'*'*5}")
cmd = "conda run --no-capture-output --live-stream -n base which python && python --version"
print(f"(BASE): {cmd}")
execute(cmd)

cmd = "conda run --no-capture-output --live-stream -n dev which python && python --version"
print(f"(DEV): {cmd}")
execute(cmd)

print(f"{'*'*5} isort version seen by conda {'*'*5}")
cmd = "conda run --no-capture-output --live-stream -n base conda list isort"
print(f"(BASE): {cmd}")
execute(cmd)

cmd = "conda run --no-capture-output --live-stream -n dev conda list isort"
print(f"(DEV): {cmd}")
execute(cmd)

print(f"{'*'*5} isort version inside python {'*'*5}")
cmd = "conda run --no-capture-output --live-stream -n base python -c 'import isort; print(isort.__version__)'"
print(f"(BASE): {cmd}")
execute(cmd)

cmd = "conda run --no-capture-output --live-stream -n dev python -c 'import isort; print(isort.__version__)'"
print(f"(DEV): {cmd}")
execute(cmd)

print(f"{'*'*5} isort version inside python - after unset PYTHONPATH{'*'*5}")
cmd = "unset PYTHONPATH && conda run --no-capture-output --live-stream -n base python -c 'import isort; print(isort.__version__)'"
print(f"(BASE): {cmd}")
execute(cmd)

cmd = "unset PYTHONPATH && conda run --no-capture-output --live-stream -n dev python -c 'import isort; print(isort.__version__)'"
print(f"(DEV): {cmd}")
execute(cmd)

print("="*100)

def execute(cmd: str):

subprocess.run(
cmd,
shell=True,
check=True,
env=os.environ.copy(),
bufsize=1,
text=True,
stderr=sys.stderr,
stdout=sys.stdout
)

if __name__ == "__main__":
demo()

```

**Expected behavior**

Either:
1. Allow virtual environments to work out of the box without polluting their module search paths.
2. If (1) isn't possible, then document the use of conda environments / virtual environments.

**Screenshots or logs**
N/A

**System information**
Versions are specified in reproduced example section.

**Additional context**
N/A

Contributor guide

Open the contributing guide

Research direction

Start at src/sagemaker_training/environment.py:1219, where sys.path is added to PYTHONPATH, and review the reproduction using problem.Dockerfile, launcher.py, and wrong_version_demo.py. Run the documented Docker and conda commands to confirm the base isort version appears in the dev environment. Done means virtual environments work without polluted module search paths, or the supported environment behavior is documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
docker, python
Domain
machine-learning, tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.