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

Runtime dependency on setuptools from importing distutils #4534

Open
jmahlik opened this issue Mar 25, 2024 · 4 comments · May be fixed by #4544 or #4837
Open

Runtime dependency on setuptools from importing distutils #4534

jmahlik opened this issue Mar 25, 2024 · 4 comments · May be fixed by #4544 or #4837
Labels
bug component: pipelines Relates to the SageMaker Pipeline Platform

Comments

@jmahlik
Copy link
Contributor

jmahlik commented Mar 25, 2024

Describe the bug
Distutils was removed from the standard library in python 3.12. Environments created under 3.12 no longer contain setuptools by default. Distutils itself is also deprecated.

There's a few usages of distutils in the sagemaker python sdk. They are easily replaced by shutil. I do see a comment in sagemaker.workflow._repack_model about not wanting to use shutil. But with the dirs_exist_ok=True parameter added in python 3.8, it wouldn't fail on existing directories. So all supported python versions now have an alternative.

# distutils.dir_util.copy_tree works way better than the half-baked
# shutil.copytree which bombs on previously existing target dirs...
# alas ... https://bugs.python.org/issue10948
# we'll go ahead and use the copy_tree function anyways because this
# repacking is some short-lived hackery, right??
from distutils.dir_util import copy_tree

git grep -e distutils
.pylintrc:ignored-modules=distutils
src/sagemaker/local/image.py:from distutils.spawn import find_executable
src/sagemaker/local/utils.py:from distutils.dir_util import copy_tree
src/sagemaker/local/utils.py:    """A wrapper around distutils.dir_util.copy_tree.
src/sagemaker/workflow/_repack_model.py:# distutils.dir_util.copy_tree works way better than the half-baked
src/sagemaker/workflow/_repack_model.py:from distutils.dir_util import copy_tree
tests/data/_repack_model.py:# distutils.dir_util.copy_tree works way better than the half-baked
tests/data/_repack_model.py:from distutils.dir_util import copy_tree
tests/unit/test_chainer.py:from distutils.util import strtobool

I'll submit a PR to remove the deprecated calls in the source.

To reproduce
A clear, step-by-step set of instructions to reproduce the bug.
The provided code need to be complete and runnable, if additional data is needed, please include them in the issue.

# No setuptools installed by default
python3.12 -m venv
# or virtualenv venv --py 3.12
. venv/bin/activate
# Install from current master
python -m pip install -e .[local]
python -c 'import sagemaker.local.image'
...
    from distutils.spawn import find_executable
  ModuleNotFoundError: No module named 'distutils'

Expected behavior
Imports do not fail.

Screenshots or logs
If applicable, add screenshots or logs to help explain your problem.

System information
A description of your system. Please provide:

  • SageMaker Python SDK version: master
  • Framework name (eg. PyTorch) or algorithm (eg. KMeans): n/a
  • Framework version: n/a
  • Python version: 3.12
  • CPU or GPU: cpu
  • Custom Docker image (Y/N): n

Additional context
Add any other context about the problem here.

@jmahlik jmahlik added the bug label Mar 25, 2024
jmahlik added a commit to StateFarmIns/sagemaker-python-sdk that referenced this issue Mar 25, 2024
@qidewenwhen qidewenwhen added the component: pipelines Relates to the SageMaker Pipeline Platform label Mar 25, 2024
jmahlik added a commit to StateFarmIns/sagemaker-python-sdk that referenced this issue Mar 27, 2024
@jmahlik jmahlik linked a pull request Mar 27, 2024 that will close this issue
9 tasks
jmahlik added a commit to StateFarmIns/sagemaker-python-sdk that referenced this issue Mar 27, 2024
jmahlik added a commit to StateFarmIns/sagemaker-python-sdk that referenced this issue Apr 8, 2024
jmahlik added a commit to StateFarmIns/sagemaker-python-sdk that referenced this issue Apr 15, 2024
@eflanagan0
Copy link

Thank you for taking the time to document and attempt fixing this @jmahlik!

As a new user of AWS SageMaker, I am running into this bug when attempting to deploy a model using the SageMaker SDK.

@eflanagan0
Copy link

relates to #3028

jmahlik added a commit to StateFarmIns/sagemaker-python-sdk that referenced this issue Jun 24, 2024
@HWiese1980
Copy link

Python 3.12 has been out for a couple months now, why is this still open? I'm currently running into exactly this problem. Python 3.12 is also already available as Lambda Runtime in AWS, so SageMaker SDK should follow as soon as possible.

@jmahlik
Copy link
Contributor Author

jmahlik commented Jul 8, 2024

See #4544 (comment). Removing the dependency causes a few integration tests to fail. Waiting on a response for how to handle that (it's complicated). There's a workaround by adding setuptools as an explicit dependency/making sure it is installed in the environment where the code is being run.

@benieric benieric linked a pull request Aug 15, 2024 that will close this issue
9 tasks
benieric pushed a commit to benieric/sagemaker-python-sdk that referenced this issue Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component: pipelines Relates to the SageMaker Pipeline Platform
Projects
None yet
4 participants