-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: actionable guidance for removed v2 interfaces #6004
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
Open
jam-jee
wants to merge
7
commits into
aws:master
Choose a base branch
from
jam-jee:feat/v3-removed-module-guidance
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+814
−1
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
9d48334
feat: actionable guidance for removed v2 interfaces
jam-jee 00e4e4c
refactor: log removed-module guidance at debug, not warning
jam-jee 15eb00b
docs: simplify fallback message to 'It may have been removed'
jam-jee aa6553a
test: assert finder pass-through via find_spec, not real import
jam-jee bfdd103
feat: point removed-interface errors at verified v3 code + docs
jam-jee 2e0e259
fix: fallback finder only guides for known removed v2 modules
jam-jee 8543eca
fix: register fallback finder on any sagemaker import, not just core
jam-jee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,16 @@ | ||
| """Namespace package for SageMaker.""" | ||
| __path__ = __import__('pkgutil').extend_path(__path__, __name__) | ||
|
|
||
| __path__ = __import__("pkgutil").extend_path(__path__, __name__) | ||
|
|
||
| # Register the fallback finder that gives actionable migration guidance for v2 | ||
| # modules removed in v3. Doing it here (in the namespace package init, which | ||
| # runs on any ``import sagemaker.*``) ensures the guidance is active even when a | ||
| # removed module is the very first sagemaker import in the process (e.g. | ||
| # ``import sagemaker.estimator``), before ``sagemaker.core`` is otherwise loaded. | ||
| # Fully guarded so it can never interfere with importing the package. | ||
| try: | ||
| from sagemaker.core.deprecations import register_removed_module_finder as _register | ||
|
|
||
| _register() | ||
| except Exception: # pylint: disable=W0703 # noqa: E722 | ||
| pass |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"). You | ||
| # may not use this file except in compliance with the License. A copy of | ||
| # the License is located at | ||
| # | ||
| # http://aws.amazon.com/apache2.0/ | ||
| # | ||
| # or in the "license" file accompanying this file. This file is | ||
| # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF | ||
| # ANY KIND, either express or implied. See the License for the specific | ||
| # language governing permissions and limitations under the License. | ||
| """Removed in v3. This module stood in the v2 SageMaker Python SDK. | ||
|
|
||
| Importing ``sagemaker.algorithm`` raises an actionable ``ModuleNotFoundError`` that | ||
| points to the v3 replacement. See the migration guide for details. | ||
| """ | ||
| from __future__ import absolute_import | ||
|
|
||
| from sagemaker.core.deprecations import raise_removed_in_v3 | ||
|
|
||
| raise_removed_in_v3( | ||
| module="sagemaker.algorithm", | ||
| replacement="`ModelTrainer`", | ||
| v3_import="from sagemaker.train import ModelTrainer", | ||
| v3_docs="https://sagemaker.readthedocs.io/en/stable/api/generated/sagemaker.train.model_trainer.html", | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"). You | ||
| # may not use this file except in compliance with the License. A copy of | ||
| # the License is located at | ||
| # | ||
| # http://aws.amazon.com/apache2.0/ | ||
| # | ||
| # or in the "license" file accompanying this file. This file is | ||
| # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF | ||
| # ANY KIND, either express or implied. See the License for the specific | ||
| # language governing permissions and limitations under the License. | ||
| """Removed in v3. This module stood in the v2 SageMaker Python SDK. | ||
|
|
||
| Importing ``sagemaker.automl`` raises an actionable ``ModuleNotFoundError`` that | ||
| points to the v3 replacement. See the migration guide for details. | ||
| """ | ||
| from __future__ import absolute_import | ||
|
|
||
| from sagemaker.core.deprecations import raise_removed_in_v3 | ||
|
|
||
| raise_removed_in_v3( | ||
| module="sagemaker.automl", | ||
| replacement="the `AutoMLJob` resource", | ||
| v3_import="from sagemaker.core.resources import AutoMLJob", | ||
| v3_docs="https://sagemaker.readthedocs.io/en/stable/api/generated/sagemaker.core.resources.html", | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"). You | ||
| # may not use this file except in compliance with the License. A copy of | ||
| # the License is located at | ||
| # | ||
| # http://aws.amazon.com/apache2.0/ | ||
| # | ||
| # or in the "license" file accompanying this file. This file is | ||
| # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF | ||
| # ANY KIND, either express or implied. See the License for the specific | ||
| # language governing permissions and limitations under the License. | ||
| """Removed in v3. This module stood in the v2 SageMaker Python SDK. | ||
|
|
||
| Importing ``sagemaker.base_predictor`` raises an actionable ``ModuleNotFoundError`` that | ||
| points to the v3 replacement. See the migration guide for details. | ||
| """ | ||
| from __future__ import absolute_import | ||
|
|
||
| from sagemaker.core.deprecations import raise_removed_in_v3 | ||
|
|
||
| raise_removed_in_v3( | ||
| module="sagemaker.base_predictor", | ||
| replacement="the `Endpoint` resource", | ||
| v3_import="from sagemaker.core.resources import Endpoint", | ||
| v3_docs="https://sagemaker.readthedocs.io/en/stable/api/generated/sagemaker.core.resources.html", | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,13 +13,236 @@ | |
| """Module for deprecation abstractions.""" | ||
| from __future__ import absolute_import | ||
|
|
||
| import importlib.abc | ||
| import logging | ||
| import sys | ||
| import warnings | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| V2_URL = "https://sagemaker.readthedocs.io/en/stable/v2.html" | ||
|
|
||
| # Migration guide for users moving from the v2 SDK to v3. | ||
| V3_MIGRATION_URL = "https://github.com/aws/sagemaker-python-sdk/blob/master/migration.md" | ||
|
|
||
| # Real top-level ``sagemaker.*`` names that ship in v3. The fallback finder must | ||
| # never intercept these -- even when a package (e.g. sagemaker-train) is simply | ||
| # not installed, its absence should surface the normal error, not a bogus | ||
| # "removed in v3" message. | ||
| _KNOWN_V3_TOPLEVEL = frozenset({"core", "train", "serve", "mlops", "lineage", "ai_registry"}) | ||
|
|
||
| # Top-level ``sagemaker.<name>`` modules that existed in v2 but were removed in | ||
| # v3 (some relocated under ``sagemaker.core.*``). Derived from the v2 top-level | ||
| # module surface (the ``master-v2`` branch), minus names that still exist in v3. | ||
| # The fallback finder only emits migration guidance for THESE names, so a typo | ||
| # or hallucinated import (e.g. ``sagemaker.foobar``) gets a plain | ||
| # ``ModuleNotFoundError`` rather than a misleading "was removed" message. V2 is | ||
| # in maintenance, so this surface is effectively frozen. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The fallback finder only emits migration guidance for THESE names, so a typoor hallucinated import (e.g.
|
||
| _REMOVED_V2_MODULES = frozenset( | ||
| { | ||
| "_studio", | ||
| "accept_types", | ||
| "algorithm", | ||
| "amazon", | ||
| "amtviz", | ||
| "analytics", | ||
| "apiutils", | ||
| "async_inference", | ||
| "automl", | ||
| "aws_batch", | ||
| "base_deserializers", | ||
| "base_predictor", | ||
| "base_serializers", | ||
| "batch_inference", | ||
| "chainer", | ||
| "clarify", | ||
| "cli", | ||
| "collection", | ||
| "compute_resource_requirements", | ||
| "config", | ||
| "container_base_model", | ||
| "content_types", | ||
| "dataset_definition", | ||
| "debugger", | ||
| "deprecations", | ||
| "deserializers", | ||
| "djl_inference", | ||
| "drift_check_baselines", | ||
| "enums", | ||
| "environment_variables", | ||
| "estimator", | ||
| "exceptions", | ||
| "experiments", | ||
| "explainer", | ||
| "feature_store", | ||
| "fw_utils", | ||
| "git_utils", | ||
| "huggingface", | ||
| "hyperparameters", | ||
| "image_uri_config", | ||
| "image_uris", | ||
| "inference_recommender", | ||
| "inputs", | ||
| "instance_group", | ||
| "instance_types", | ||
| "instance_types_gpu_info", | ||
| "interactive_apps", | ||
| "iterators", | ||
| "job", | ||
| "jumpstart", | ||
| "lambda_helper", | ||
| "local", | ||
| "logs", | ||
| "metadata_properties", | ||
| "metric_definitions", | ||
| "mlflow", | ||
| "model", | ||
| "model_card", | ||
| "model_life_cycle", | ||
| "model_metrics", | ||
| "model_monitor", | ||
| "model_uris", | ||
| "modules", | ||
| "multidatamodel", | ||
| "mxnet", | ||
| "network", | ||
| "parameter", | ||
| "partner_app", | ||
| "payloads", | ||
| "pipeline", | ||
| "predictor", | ||
| "predictor_async", | ||
| "processing", | ||
| "pytorch", | ||
| "remote_function", | ||
| "resource_requirements", | ||
| "rl", | ||
| "s3", | ||
| "s3_utils", | ||
| "script_uris", | ||
| "serializer_utils", | ||
| "serializers", | ||
| "serverless", | ||
| "session", | ||
| "session_settings", | ||
| "sklearn", | ||
| "spark", | ||
| "sparkml", | ||
| "stabilityai", | ||
| "telemetry", | ||
| "tensorflow", | ||
| "training_compiler", | ||
| "transformer", | ||
| "tuner", | ||
| "user_agent", | ||
| "utilities", | ||
| "utils", | ||
| "vpc_utils", | ||
| "workflow", | ||
| "wrangler", | ||
| "xgboost", | ||
| } | ||
| ) | ||
|
|
||
|
|
||
| def raise_removed_in_v3(module, replacement=None, v3_import=None, v3_docs=None): | ||
| """Warn and then raise an actionable error for a v2 module removed in v3. | ||
|
|
||
| The v2 SDK exposed top-level modules (e.g. ``sagemaker.estimator``) that no | ||
| longer exist in v3. Importing one would otherwise fail with a bare | ||
| ``ModuleNotFoundError: No module named 'sagemaker.estimator'`` that gives the | ||
| caller no path forward. This helper is called from lightweight "tombstone" | ||
| modules that stand in for those removed names: it emits a | ||
| ``DeprecationWarning`` and then raises a ``ModuleNotFoundError`` whose message | ||
| names the exact v3 replacement, the import to copy-paste, and a direct link | ||
| to that replacement's API docs (plus the migration guide). | ||
|
|
||
| Args: | ||
| module (str): The removed v2 module path, e.g. ``"sagemaker.estimator"``. | ||
| replacement (str): Human readable v3 replacement, e.g. ``"ModelTrainer"``. | ||
| Optional. | ||
| v3_import (str): The exact v3 import statement, e.g. | ||
| ``"from sagemaker.train import ModelTrainer"``. Quoted verbatim so the | ||
| caller can copy-paste it. Optional. | ||
| v3_docs (str): Direct URL to the v3 replacement's API documentation, e.g. | ||
| the generated ``sagemaker.train.model_trainer`` page. Optional. | ||
|
|
||
| Raises: | ||
| ModuleNotFoundError: always, after emitting the deprecation warning. | ||
| """ | ||
| msg = f"`{module}` was removed in the SageMaker Python SDK v3." | ||
| if replacement: | ||
| msg += f" Use {replacement}." | ||
| if v3_import: | ||
| msg += f" ({v3_import})" | ||
| if v3_docs: | ||
| msg += f"\nDocs: {v3_docs}" | ||
| msg += f"\nSee {V3_MIGRATION_URL} for the migration guide." | ||
|
|
||
| warnings.warn(msg, DeprecationWarning, stacklevel=2) | ||
| # The raised ModuleNotFoundError below is the loud, authoritative signal | ||
| # (it stops execution and carries the full message). Log at debug only, to | ||
| # leave a breadcrumb for log-captured environments without duplicating the | ||
| # message at WARNING level. | ||
| logger.debug(msg) | ||
| raise ModuleNotFoundError(msg, name=module) | ||
|
|
||
|
|
||
| class _RemovedV2ModuleFinder(importlib.abc.MetaPathFinder): | ||
| """Fallback finder that gives actionable guidance for removed v2 modules. | ||
|
|
||
| Curated removals ship as explicit "tombstone" modules (e.g. | ||
| ``sagemaker/estimator.py``) that raise a precise, per-module message. This | ||
| finder is the *catch-all* for every other ``sagemaker.<name>`` that existed | ||
| in v2 but was not individually tombstoned: instead of a bare | ||
| ``ModuleNotFoundError: No module named 'sagemaker.foo'``, the caller gets a | ||
| message that says the module is not available in v3 and points to the | ||
| migration guide. | ||
|
|
||
| It is registered by appending to ``sys.meta_path``, so it only runs *after* | ||
| the normal import machinery has failed to locate the module. That ordering | ||
| guarantees it never shadows: | ||
| - real v3 packages (``sagemaker.core``, ``sagemaker.train``, ...), and | ||
| - the curated tombstone modules, which resolve as ordinary files first. | ||
| """ | ||
|
|
||
| def find_spec(self, fullname, path=None, target=None): | ||
| """Emit guidance only for known removed v2 top-level ``sagemaker`` modules.""" | ||
| if not fullname.startswith("sagemaker."): | ||
| return None | ||
| leaf = fullname[len("sagemaker.") :] | ||
| # Only guard top-level names; never touch real v3 subpackages. | ||
| if "." in leaf or leaf in _KNOWN_V3_TOPLEVEL: | ||
| return None | ||
| # Only guard names that were actually v2 modules. Unknown names (typos, | ||
| # hallucinated imports) fall through to a plain ModuleNotFoundError so we | ||
| # never claim something "was removed" when it never existed. | ||
| if leaf not in _REMOVED_V2_MODULES: | ||
| return None | ||
|
|
||
| msg = ( | ||
| f"`{fullname}` was removed in the SageMaker Python SDK v3. " | ||
| "It may have moved to a new location." | ||
| f"\nSee {V3_MIGRATION_URL} for the migration guide." | ||
| ) | ||
| warnings.warn(msg, DeprecationWarning, stacklevel=2) | ||
| # See raise_removed_in_v3: the raised error is the loud signal; log at | ||
| # debug to avoid duplicating the message at WARNING level. | ||
| logger.debug(msg) | ||
| raise ModuleNotFoundError(msg, name=fullname) | ||
|
|
||
|
|
||
| def register_removed_module_finder(): | ||
| """Install the fallback finder for removed v2 modules (idempotent). | ||
|
|
||
| Appends a single ``_RemovedV2ModuleFinder`` to ``sys.meta_path`` so it acts | ||
| as a last resort. Safe to call multiple times -- it installs at most one | ||
| instance per process. | ||
| """ | ||
| if any(isinstance(f, _RemovedV2ModuleFinder) for f in sys.meta_path): | ||
| return | ||
|
Comment on lines
+242
to
+243
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add a info log here? |
||
| sys.meta_path.append(_RemovedV2ModuleFinder()) | ||
|
|
||
|
|
||
| def _warn(msg, sdk_version=None): | ||
| """Generic warning raiser referencing V2 | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"). You | ||
| # may not use this file except in compliance with the License. A copy of | ||
| # the License is located at | ||
| # | ||
| # http://aws.amazon.com/apache2.0/ | ||
| # | ||
| # or in the "license" file accompanying this file. This file is | ||
| # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF | ||
| # ANY KIND, either express or implied. See the License for the specific | ||
| # language governing permissions and limitations under the License. | ||
| """Removed in v3. This module stood in the v2 SageMaker Python SDK. | ||
|
|
||
| Importing ``sagemaker.estimator`` raises an actionable ``ModuleNotFoundError`` that | ||
| points to the v3 replacement. See the migration guide for details. | ||
| """ | ||
| from __future__ import absolute_import | ||
|
|
||
| from sagemaker.core.deprecations import raise_removed_in_v3 | ||
|
|
||
| raise_removed_in_v3( | ||
| module="sagemaker.estimator", | ||
| replacement="`ModelTrainer`", | ||
| v3_import="from sagemaker.train import ModelTrainer", | ||
| v3_docs="https://sagemaker.readthedocs.io/en/stable/api/generated/sagemaker.train.model_trainer.html", | ||
| ) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need these files as well? can we not infer which import is failing it the
find_specagain and returng appropriate error mesage? In other words, what is the value addition of having these files?