Skip to content

Conversation

jameslamb
Copy link
Member

@jameslamb jameslamb commented Jan 14, 2025

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Details

Proposes updating to v2.237.3, the latest version on PyPI (https://pypi.org/project/sagemaker/#sagemaker-2.237.3.tar.gz).

And relaxing the package's pin on cloudpickle, to match the changes from aws/sagemaker-python-sdk#4964.

Also pulls over changes from #314 , based on the bot's suggestion at #315 (comment)

@jameslamb
Copy link
Member Author

@conda-forge-admin, please rerender

@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

  • ❌ The conda-forge.yml file is not allowed to have duplicate keys.

For recipe/meta.yaml:

  • ℹ️ noarch: python recipes should usually follow the syntax in our documentation for specifying the Python version.
    • For the host section of the recipe, you should usually use python {{ python_min }} for the python entry.
    • For the run section of the recipe, you should usually use python >={{ python_min }} for the python entry.
    • For the test.requires section of the recipe, you should usually use python {{ python_min }} for the python entry.
    • If the package requires a newer Python version than the currently supported minimum version on conda-forge, you can override the python_min variable by adding a Jinja2 set statement at the top of your recipe (or using an equivalent context variable for v1 recipes).
  • ℹ️ PyPI default URL is now pypi.org, and not pypi.io. You may want to update the default source url.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12776370400. Examine the logs at this URL for more detail.

conda-forge-webservices[bot] and others added 3 commits January 14, 2025 21:08
@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Jan 14, 2025

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe/meta.yaml:

  • ℹ️ noarch: python recipes should usually follow the syntax in our documentation for specifying the Python version.
    • For the host section of the recipe, you should usually use python {{ python_min }} for the python entry.
    • For the run section of the recipe, you should usually use python >={{ python_min }} for the python entry.
    • For the test.requires section of the recipe, you should usually use python {{ python_min }} for the python entry.
    • If the package requires a newer Python version than the currently supported minimum version on conda-forge, you can override the python_min variable by adding a Jinja2 set statement at the top of your recipe (or using an equivalent context variable for v1 recipes).
  • ℹ️ PyPI default URL is now pypi.org, and not pypi.io. You may want to update the default source url.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12776416588. Examine the logs at this URL for more detail.

@jameslamb
Copy link
Member Author

@conda-forge-admin, please rerender

@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

@jameslamb
Copy link
Member Author

Import tests are failing like this:

import: 'sagemaker'
Traceback (most recent call last):
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/test_tmp/run_test.py", line 2, in <module>
    import sagemaker
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib/python3.9/site-packages/sagemaker/__init__.py", line 18, in <module>
    from sagemaker import estimator, parameter, tuner  # noqa: F401
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib/python3.9/site-packages/sagemaker/estimator.py", line 30, in <module>
    from sagemaker import git_utils, image_uris, vpc_utils, s3
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib/python3.9/site-packages/sagemaker/image_uris.py", line 24, in <module>
    from sagemaker.jumpstart.constants import DEFAULT_JUMPSTART_SAGEMAKER_SESSION, JUMPSTART_LOGGER
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib/python3.9/site-packages/sagemaker/jumpstart/constants.py", line 27, in <module>
    from sagemaker.jumpstart.types import JumpStartLaunchedRegionInfo, JumpStartS3FileType
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib/python3.9/site-packages/sagemaker/jumpstart/types.py", line 20, in <module>
    from sagemaker_core.shapes import ModelAccessConfig as CoreModelAccessConfig
ModuleNotFoundError: No module named 'sagemaker_core'

(build link)

It appears that some bits of this library were recently broken out into a separate package called sagemaker-core: https://pypi.org/project/sagemaker-core/.

There doesn't appear to be a package by that name yet on conda-forge. Hoping a repo maintainer here could add that, following the steps at https://conda-forge.org/docs/maintainer/adding_pkgs/, or suggest some other way around this.

@claytonparnell
Copy link
Contributor

Thanks for doing this @jameslamb . Are you looking into the build failures? It seems like we need to have https://pypi.org/project/sagemaker-core/ published to conda-forge as well and have that added as a dependency. I can reach out to sagemaker-python-sdk team for this.

Currently failing with

import: 'sagemaker'
Traceback (most recent call last):
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/test_tmp/run_test.py", line 2, in <module>
    import sagemaker
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib/python3.9/site-packages/sagemaker/__init__.py", line 18, in <module>
    from sagemaker import estimator, parameter, tuner  # noqa: F401
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib/python3.9/site-packages/sagemaker/estimator.py", line 30, in <module>
    from sagemaker import git_utils, image_uris, vpc_utils, s3
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib/python3.9/site-packages/sagemaker/image_uris.py", line 24, in <module>
    from sagemaker.jumpstart.constants import DEFAULT_JUMPSTART_SAGEMAKER_SESSION, JUMPSTART_LOGGER
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib/python3.9/site-packages/sagemaker/jumpstart/constants.py", line 27, in <module>
    from sagemaker.jumpstart.types import JumpStartLaunchedRegionInfo, JumpStartS3FileType
  File "/home/conda/feedstock_root/build_artifacts/sagemaker-python-sdk_1736889353132/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/lib/python3.9/site-packages/sagemaker/jumpstart/types.py", line 20, in <module>
    from sagemaker_core.shapes import ModelAccessConfig as CoreModelAccessConfig
ModuleNotFoundError: No module named 'sagemaker_core'

@claytonparnell
Copy link
Contributor

I just saw you commented basically the same thing right before me 😄 working on getting sagemaker-core added to conda-forge.

@jakirkham
Copy link
Member

So there are 2 things we can consider:

  1. Add new packages to conda-forge
  2. Turn this (or an appropriate AWS) feedstock into one that produces multiple packages

If this is a totally different package and there isn't much overlap, 1 can be a good choice

If there is more overlap (like shared dependencies with common pinning needs), 2 can be a reasonable choice as it keeps all the pieces in one place

James as you have dug into this more, do you have a sense of which case it sounds more like?

@claytonparnell
Copy link
Contributor

sagemaker-python-sdk dependencies: https://github.com/aws/sagemaker-python-sdk/blob/master/pyproject.toml#L33-L59
sagemaker-core dependencies: https://github.com/aws/sagemaker-core/blob/main/pyproject.toml#L13-L23

I was thinking since they have different dependencies and different release cadence/versioning, it would make sense to have separate feedstocks. Though I don't have a ton of experience here in managing more complex conda-forge feedstocks.

I created conda-forge/staged-recipes#28839 for adding sagemaker-core recipe.

@jakirkham
Copy link
Member

Thanks Clayton and James! 🙏

Let's try restarting now that the package is up and on CDN

@conda-forge-admin , please restart CI

@claytonparnell
Copy link
Contributor

Just updated your branch as well to add sagemaker-core and update other dependencies

@claytonparnell claytonparnell merged commit af75080 into conda-forge:main Jan 15, 2025
4 checks passed
@jameslamb jameslamb deleted the release-v2.237.3 branch January 15, 2025 21:02
@jameslamb
Copy link
Member Author

thanks so much for this @claytonparnell !!!

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

Successfully merging this pull request may close these issues.

4 participants