fix(serve): make torch an optional dependency - #6166

Open
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve
Open

fix(serve): make torch an optional dependency#6166
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve

Conversation

@goelakash

Copy link
Copy Markdown
Contributor

Fixes#5531

sagemaker-serve lists torch>=2.0.0 as a required dependency, so any install of sagemaker, sagemaker-serve, or sagemaker-mlops pulls torch (and on Linux, the CUDA/cuDNN/NCCL stack) even for API-only use. sagemaker-core already treats torch as an extra.

The blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in serve/constants.py, which instantiated TorchTensorSerializer() at module scope. That runs from torch import Tensor on any import sagemaker.serve. Every other torch reference in the package is already lazy.

Changes

  • Store serializer/deserializer classes in DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate at lookup in _fetch_serializer_and_deserializer_for_framework.
  • Move torch>=2.0.0 to a torch extra, matching sagemaker-core.
  • Update the two docstring examples and the affected unit tests.
  • Add tests/unit/test_optional_torch_dependency.py, mirroring the existing sagemaker-core subprocess pattern.

Installing with the torch extra is unchanged. TorchTensorSerializer() still raises the same ImportError if torch is missing when actually used.

Testing

  • tests/unit in a venv with and without torch: no new failures vs. master (53 pre-existing failures in both).
  • New tests fail on master and pass with this change.
  • Verified in a container with no torch installed: from sagemaker.serve import ModelBuilder and import sagemaker.mlops both succeed.
  • Image size for all four packages on Amazon Linux 2023: 1.45 GB -> 536 MB.

Installing sagemaker-serve (or the umbrella sagemaker, which depends on
it) pulled torch, and on Linux the ~2.9 GB CUDA/cuDNN/NCCL closure, even
for API-only use. sagemaker-core already declared torch an extra;
sagemaker-serve declared it required, so serve users paid regardless.
The import-time blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in
serve/constants.py, which instantiated TorchTensorSerializer() at module
scope, running `from torch import Tensor` on any `import sagemaker.serve`.
Changes:
- Store serializer/deserializer classes (not instances) in
DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate on lookup. Removes the
import-time torch dependency.
- Move torch>=2.0.0 to a `torch` extra in sagemaker-serve, matching
sagemaker-core. Add a `torch` extra to the umbrella sagemaker package
(sagemaker-serve[torch]) so `pip install sagemaker[torch]` works.
- Duck-type the tensor check in TorchTensorSerializer: serialization only
needs the object's own detach()/numpy(), so it no longer imports torch
at all and works whenever the caller holds a tensor.
- Make the in-process model server's torch import lazy with a CPU
fallback, so importing it no longer requires torch.
- Fix the error messages that told serve users to install the wrong
package: TorchTensorDeserializer, the Triton translator, and the ONNX
export path now name sagemaker-serve[torch].
- Update and add tests asserting the whole surface imports and serializes
without torch, and that the paths that genuinely construct torch
objects (deserializer, Triton translator) still raise a clear error
naming the extra.
Fixesaws#5531
Breaking change: users who installed plain sagemaker-serve and relied on
a torch code path (deserializing to tensor/pt, or ONNX/Triton export of a
PyTorch model) must now install sagemaker-serve[torch]. Both paths
inherently require a torch object the caller supplies, so torch is
already present in practice; a GitHub-wide search found no external
callers of these paths that do not already import torch.
---
X-AI-Prompt: address the reviewer concern on whether making torch optional breaks existing customers; find ways to reduce the torch dependency and update pyproject and docs; consolidate all changes in the goelakash93 fork for a possible major version bump.
X-AI-Tool: Claude Code
@goelakash
goelakashforce-pushed the fix-optional-torch-serve branch from 38b75f5 to 299438eCompareAugust 7, 2026 20:49
@goelakash
goelakashdeployed to manual-approval August 7, 2026 20:49 — with GitHub Actions Active
jam-jee added a commit that referenced this pull request Aug 17, 2026
The reviewer aborted on every fork PR with "Actor does not have write
permissions to the repository" (e.g. run 31217496013 on #6166), so it
only ever ran for collaborators.
claude-code-action checks that the PR author has write access before
doing anything. That default protects its normal @claude usage, where a
read-only user's comment becomes the prompt. It does not apply here:
pull_request_target always runs the base-branch copy of this workflow,
so the prompt is fixed by maintainers and a fork cannot supply it.
Set allowed_non_write_users so the review actually runs, and harden the
prompt-injection surface it exposes (untrusted diff/PR text entering
context):
- deny Read on /proc, /sys, ~/.aws, the Actions _temp dir and .git/config
so an injected instruction cannot use the review comment as a
secret-exfiltration channel
- instruct the model to treat all contributor-authored content as data,
never as instructions, and to report attempted injection
Fork PRs continue to require maintainer approval via the manual-approval
environment, Bash/Write/Edit remain unavailable, and the assumed role is
still limited to bedrock:InvokeModel on a single inference profile.
@goelakash
goelakashdeployed to manual-approval August 18, 2026 20:42 — with GitHub Actions Active
Sign up for freeto 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.

Make torch an optional dependency / extra

1 participant

@goelakash
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(serve): make torch an optional dependency - #6166

Open
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve
Open

fix(serve): make torch an optional dependency#6166
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve

Conversation

@goelakash

Copy link
Copy Markdown
Contributor

Fixes#5531

sagemaker-serve lists torch>=2.0.0 as a required dependency, so any install of sagemaker, sagemaker-serve, or sagemaker-mlops pulls torch (and on Linux, the CUDA/cuDNN/NCCL stack) even for API-only use. sagemaker-core already treats torch as an extra.

The blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in serve/constants.py, which instantiated TorchTensorSerializer() at module scope. That runs from torch import Tensor on any import sagemaker.serve. Every other torch reference in the package is already lazy.

Changes

  • Store serializer/deserializer classes in DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate at lookup in _fetch_serializer_and_deserializer_for_framework.
  • Move torch>=2.0.0 to a torch extra, matching sagemaker-core.
  • Update the two docstring examples and the affected unit tests.
  • Add tests/unit/test_optional_torch_dependency.py, mirroring the existing sagemaker-core subprocess pattern.

Installing with the torch extra is unchanged. TorchTensorSerializer() still raises the same ImportError if torch is missing when actually used.

Testing

  • tests/unit in a venv with and without torch: no new failures vs. master (53 pre-existing failures in both).
  • New tests fail on master and pass with this change.
  • Verified in a container with no torch installed: from sagemaker.serve import ModelBuilder and import sagemaker.mlops both succeed.
  • Image size for all four packages on Amazon Linux 2023: 1.45 GB -> 536 MB.

Installing sagemaker-serve (or the umbrella sagemaker, which depends on
it) pulled torch, and on Linux the ~2.9 GB CUDA/cuDNN/NCCL closure, even
for API-only use. sagemaker-core already declared torch an extra;
sagemaker-serve declared it required, so serve users paid regardless.
The import-time blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in
serve/constants.py, which instantiated TorchTensorSerializer() at module
scope, running `from torch import Tensor` on any `import sagemaker.serve`.
Changes:
- Store serializer/deserializer classes (not instances) in
DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate on lookup. Removes the
import-time torch dependency.
- Move torch>=2.0.0 to a `torch` extra in sagemaker-serve, matching
sagemaker-core. Add a `torch` extra to the umbrella sagemaker package
(sagemaker-serve[torch]) so `pip install sagemaker[torch]` works.
- Duck-type the tensor check in TorchTensorSerializer: serialization only
needs the object's own detach()/numpy(), so it no longer imports torch
at all and works whenever the caller holds a tensor.
- Make the in-process model server's torch import lazy with a CPU
fallback, so importing it no longer requires torch.
- Fix the error messages that told serve users to install the wrong
package: TorchTensorDeserializer, the Triton translator, and the ONNX
export path now name sagemaker-serve[torch].
- Update and add tests asserting the whole surface imports and serializes
without torch, and that the paths that genuinely construct torch
objects (deserializer, Triton translator) still raise a clear error
naming the extra.
Fixesaws#5531
Breaking change: users who installed plain sagemaker-serve and relied on
a torch code path (deserializing to tensor/pt, or ONNX/Triton export of a
PyTorch model) must now install sagemaker-serve[torch]. Both paths
inherently require a torch object the caller supplies, so torch is
already present in practice; a GitHub-wide search found no external
callers of these paths that do not already import torch.
---
X-AI-Prompt: address the reviewer concern on whether making torch optional breaks existing customers; find ways to reduce the torch dependency and update pyproject and docs; consolidate all changes in the goelakash93 fork for a possible major version bump.
X-AI-Tool: Claude Code
@goelakash
goelakashforce-pushed the fix-optional-torch-serve branch from 38b75f5 to 299438eCompareAugust 7, 2026 20:49
@goelakash
goelakashdeployed to manual-approval August 7, 2026 20:49 — with GitHub Actions Active
jam-jee added a commit that referenced this pull request Aug 17, 2026
The reviewer aborted on every fork PR with "Actor does not have write
permissions to the repository" (e.g. run 31217496013 on #6166), so it
only ever ran for collaborators.
claude-code-action checks that the PR author has write access before
doing anything. That default protects its normal @claude usage, where a
read-only user's comment becomes the prompt. It does not apply here:
pull_request_target always runs the base-branch copy of this workflow,
so the prompt is fixed by maintainers and a fork cannot supply it.
Set allowed_non_write_users so the review actually runs, and harden the
prompt-injection surface it exposes (untrusted diff/PR text entering
context):
- deny Read on /proc, /sys, ~/.aws, the Actions _temp dir and .git/config
so an injected instruction cannot use the review comment as a
secret-exfiltration channel
- instruct the model to treat all contributor-authored content as data,
never as instructions, and to report attempted injection
Fork PRs continue to require maintainer approval via the manual-approval
environment, Bash/Write/Edit remain unavailable, and the assumed role is
still limited to bedrock:InvokeModel on a single inference profile.
@goelakash
goelakashdeployed to manual-approval August 18, 2026 20:42 — with GitHub Actions Active
Sign up for freeto 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.

Make torch an optional dependency / extra

1 participant

@goelakash
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(serve): make torch an optional dependency - #6166

Open
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve
Open

fix(serve): make torch an optional dependency#6166
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve

Conversation

@goelakash

Copy link
Copy Markdown
Contributor

Fixes#5531

sagemaker-serve lists torch>=2.0.0 as a required dependency, so any install of sagemaker, sagemaker-serve, or sagemaker-mlops pulls torch (and on Linux, the CUDA/cuDNN/NCCL stack) even for API-only use. sagemaker-core already treats torch as an extra.

The blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in serve/constants.py, which instantiated TorchTensorSerializer() at module scope. That runs from torch import Tensor on any import sagemaker.serve. Every other torch reference in the package is already lazy.

Changes

  • Store serializer/deserializer classes in DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate at lookup in _fetch_serializer_and_deserializer_for_framework.
  • Move torch>=2.0.0 to a torch extra, matching sagemaker-core.
  • Update the two docstring examples and the affected unit tests.
  • Add tests/unit/test_optional_torch_dependency.py, mirroring the existing sagemaker-core subprocess pattern.

Installing with the torch extra is unchanged. TorchTensorSerializer() still raises the same ImportError if torch is missing when actually used.

Testing

  • tests/unit in a venv with and without torch: no new failures vs. master (53 pre-existing failures in both).
  • New tests fail on master and pass with this change.
  • Verified in a container with no torch installed: from sagemaker.serve import ModelBuilder and import sagemaker.mlops both succeed.
  • Image size for all four packages on Amazon Linux 2023: 1.45 GB -> 536 MB.

Installing sagemaker-serve (or the umbrella sagemaker, which depends on
it) pulled torch, and on Linux the ~2.9 GB CUDA/cuDNN/NCCL closure, even
for API-only use. sagemaker-core already declared torch an extra;
sagemaker-serve declared it required, so serve users paid regardless.
The import-time blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in
serve/constants.py, which instantiated TorchTensorSerializer() at module
scope, running `from torch import Tensor` on any `import sagemaker.serve`.
Changes:
- Store serializer/deserializer classes (not instances) in
DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate on lookup. Removes the
import-time torch dependency.
- Move torch>=2.0.0 to a `torch` extra in sagemaker-serve, matching
sagemaker-core. Add a `torch` extra to the umbrella sagemaker package
(sagemaker-serve[torch]) so `pip install sagemaker[torch]` works.
- Duck-type the tensor check in TorchTensorSerializer: serialization only
needs the object's own detach()/numpy(), so it no longer imports torch
at all and works whenever the caller holds a tensor.
- Make the in-process model server's torch import lazy with a CPU
fallback, so importing it no longer requires torch.
- Fix the error messages that told serve users to install the wrong
package: TorchTensorDeserializer, the Triton translator, and the ONNX
export path now name sagemaker-serve[torch].
- Update and add tests asserting the whole surface imports and serializes
without torch, and that the paths that genuinely construct torch
objects (deserializer, Triton translator) still raise a clear error
naming the extra.
Fixesaws#5531
Breaking change: users who installed plain sagemaker-serve and relied on
a torch code path (deserializing to tensor/pt, or ONNX/Triton export of a
PyTorch model) must now install sagemaker-serve[torch]. Both paths
inherently require a torch object the caller supplies, so torch is
already present in practice; a GitHub-wide search found no external
callers of these paths that do not already import torch.
---
X-AI-Prompt: address the reviewer concern on whether making torch optional breaks existing customers; find ways to reduce the torch dependency and update pyproject and docs; consolidate all changes in the goelakash93 fork for a possible major version bump.
X-AI-Tool: Claude Code
@goelakash
goelakashforce-pushed the fix-optional-torch-serve branch from 38b75f5 to 299438eCompareAugust 7, 2026 20:49
@goelakash
goelakashdeployed to manual-approval August 7, 2026 20:49 — with GitHub Actions Active
jam-jee added a commit that referenced this pull request Aug 17, 2026
The reviewer aborted on every fork PR with "Actor does not have write
permissions to the repository" (e.g. run 31217496013 on #6166), so it
only ever ran for collaborators.
claude-code-action checks that the PR author has write access before
doing anything. That default protects its normal @claude usage, where a
read-only user's comment becomes the prompt. It does not apply here:
pull_request_target always runs the base-branch copy of this workflow,
so the prompt is fixed by maintainers and a fork cannot supply it.
Set allowed_non_write_users so the review actually runs, and harden the
prompt-injection surface it exposes (untrusted diff/PR text entering
context):
- deny Read on /proc, /sys, ~/.aws, the Actions _temp dir and .git/config
so an injected instruction cannot use the review comment as a
secret-exfiltration channel
- instruct the model to treat all contributor-authored content as data,
never as instructions, and to report attempted injection
Fork PRs continue to require maintainer approval via the manual-approval
environment, Bash/Write/Edit remain unavailable, and the assumed role is
still limited to bedrock:InvokeModel on a single inference profile.
@goelakash
goelakashdeployed to manual-approval August 18, 2026 20:42 — with GitHub Actions Active
Sign up for freeto 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.

Make torch an optional dependency / extra

1 participant

@goelakash
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(serve): make torch an optional dependency - #6166

Open
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve
Open

fix(serve): make torch an optional dependency#6166
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve

Conversation

@goelakash

Copy link
Copy Markdown
Contributor

Fixes#5531

sagemaker-serve lists torch>=2.0.0 as a required dependency, so any install of sagemaker, sagemaker-serve, or sagemaker-mlops pulls torch (and on Linux, the CUDA/cuDNN/NCCL stack) even for API-only use. sagemaker-core already treats torch as an extra.

The blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in serve/constants.py, which instantiated TorchTensorSerializer() at module scope. That runs from torch import Tensor on any import sagemaker.serve. Every other torch reference in the package is already lazy.

Changes

  • Store serializer/deserializer classes in DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate at lookup in _fetch_serializer_and_deserializer_for_framework.
  • Move torch>=2.0.0 to a torch extra, matching sagemaker-core.
  • Update the two docstring examples and the affected unit tests.
  • Add tests/unit/test_optional_torch_dependency.py, mirroring the existing sagemaker-core subprocess pattern.

Installing with the torch extra is unchanged. TorchTensorSerializer() still raises the same ImportError if torch is missing when actually used.

Testing

  • tests/unit in a venv with and without torch: no new failures vs. master (53 pre-existing failures in both).
  • New tests fail on master and pass with this change.
  • Verified in a container with no torch installed: from sagemaker.serve import ModelBuilder and import sagemaker.mlops both succeed.
  • Image size for all four packages on Amazon Linux 2023: 1.45 GB -> 536 MB.

Installing sagemaker-serve (or the umbrella sagemaker, which depends on
it) pulled torch, and on Linux the ~2.9 GB CUDA/cuDNN/NCCL closure, even
for API-only use. sagemaker-core already declared torch an extra;
sagemaker-serve declared it required, so serve users paid regardless.
The import-time blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in
serve/constants.py, which instantiated TorchTensorSerializer() at module
scope, running `from torch import Tensor` on any `import sagemaker.serve`.
Changes:
- Store serializer/deserializer classes (not instances) in
DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate on lookup. Removes the
import-time torch dependency.
- Move torch>=2.0.0 to a `torch` extra in sagemaker-serve, matching
sagemaker-core. Add a `torch` extra to the umbrella sagemaker package
(sagemaker-serve[torch]) so `pip install sagemaker[torch]` works.
- Duck-type the tensor check in TorchTensorSerializer: serialization only
needs the object's own detach()/numpy(), so it no longer imports torch
at all and works whenever the caller holds a tensor.
- Make the in-process model server's torch import lazy with a CPU
fallback, so importing it no longer requires torch.
- Fix the error messages that told serve users to install the wrong
package: TorchTensorDeserializer, the Triton translator, and the ONNX
export path now name sagemaker-serve[torch].
- Update and add tests asserting the whole surface imports and serializes
without torch, and that the paths that genuinely construct torch
objects (deserializer, Triton translator) still raise a clear error
naming the extra.
Fixesaws#5531
Breaking change: users who installed plain sagemaker-serve and relied on
a torch code path (deserializing to tensor/pt, or ONNX/Triton export of a
PyTorch model) must now install sagemaker-serve[torch]. Both paths
inherently require a torch object the caller supplies, so torch is
already present in practice; a GitHub-wide search found no external
callers of these paths that do not already import torch.
---
X-AI-Prompt: address the reviewer concern on whether making torch optional breaks existing customers; find ways to reduce the torch dependency and update pyproject and docs; consolidate all changes in the goelakash93 fork for a possible major version bump.
X-AI-Tool: Claude Code
@goelakash
goelakashforce-pushed the fix-optional-torch-serve branch from 38b75f5 to 299438eCompareAugust 7, 2026 20:49
@goelakash
goelakashdeployed to manual-approval August 7, 2026 20:49 — with GitHub Actions Active
jam-jee added a commit that referenced this pull request Aug 17, 2026
The reviewer aborted on every fork PR with "Actor does not have write
permissions to the repository" (e.g. run 31217496013 on #6166), so it
only ever ran for collaborators.
claude-code-action checks that the PR author has write access before
doing anything. That default protects its normal @claude usage, where a
read-only user's comment becomes the prompt. It does not apply here:
pull_request_target always runs the base-branch copy of this workflow,
so the prompt is fixed by maintainers and a fork cannot supply it.
Set allowed_non_write_users so the review actually runs, and harden the
prompt-injection surface it exposes (untrusted diff/PR text entering
context):
- deny Read on /proc, /sys, ~/.aws, the Actions _temp dir and .git/config
so an injected instruction cannot use the review comment as a
secret-exfiltration channel
- instruct the model to treat all contributor-authored content as data,
never as instructions, and to report attempted injection
Fork PRs continue to require maintainer approval via the manual-approval
environment, Bash/Write/Edit remain unavailable, and the assumed role is
still limited to bedrock:InvokeModel on a single inference profile.
@goelakash
goelakashdeployed to manual-approval August 18, 2026 20:42 — with GitHub Actions Active
Sign up for freeto 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.

Make torch an optional dependency / extra

1 participant

@goelakash
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(serve): make torch an optional dependency - #6166

Open
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve
Open

fix(serve): make torch an optional dependency#6166
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve

Conversation

@goelakash

Copy link
Copy Markdown
Contributor

Fixes#5531

sagemaker-serve lists torch>=2.0.0 as a required dependency, so any install of sagemaker, sagemaker-serve, or sagemaker-mlops pulls torch (and on Linux, the CUDA/cuDNN/NCCL stack) even for API-only use. sagemaker-core already treats torch as an extra.

The blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in serve/constants.py, which instantiated TorchTensorSerializer() at module scope. That runs from torch import Tensor on any import sagemaker.serve. Every other torch reference in the package is already lazy.

Changes

  • Store serializer/deserializer classes in DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate at lookup in _fetch_serializer_and_deserializer_for_framework.
  • Move torch>=2.0.0 to a torch extra, matching sagemaker-core.
  • Update the two docstring examples and the affected unit tests.
  • Add tests/unit/test_optional_torch_dependency.py, mirroring the existing sagemaker-core subprocess pattern.

Installing with the torch extra is unchanged. TorchTensorSerializer() still raises the same ImportError if torch is missing when actually used.

Testing

  • tests/unit in a venv with and without torch: no new failures vs. master (53 pre-existing failures in both).
  • New tests fail on master and pass with this change.
  • Verified in a container with no torch installed: from sagemaker.serve import ModelBuilder and import sagemaker.mlops both succeed.
  • Image size for all four packages on Amazon Linux 2023: 1.45 GB -> 536 MB.

Installing sagemaker-serve (or the umbrella sagemaker, which depends on
it) pulled torch, and on Linux the ~2.9 GB CUDA/cuDNN/NCCL closure, even
for API-only use. sagemaker-core already declared torch an extra;
sagemaker-serve declared it required, so serve users paid regardless.
The import-time blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in
serve/constants.py, which instantiated TorchTensorSerializer() at module
scope, running `from torch import Tensor` on any `import sagemaker.serve`.
Changes:
- Store serializer/deserializer classes (not instances) in
DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate on lookup. Removes the
import-time torch dependency.
- Move torch>=2.0.0 to a `torch` extra in sagemaker-serve, matching
sagemaker-core. Add a `torch` extra to the umbrella sagemaker package
(sagemaker-serve[torch]) so `pip install sagemaker[torch]` works.
- Duck-type the tensor check in TorchTensorSerializer: serialization only
needs the object's own detach()/numpy(), so it no longer imports torch
at all and works whenever the caller holds a tensor.
- Make the in-process model server's torch import lazy with a CPU
fallback, so importing it no longer requires torch.
- Fix the error messages that told serve users to install the wrong
package: TorchTensorDeserializer, the Triton translator, and the ONNX
export path now name sagemaker-serve[torch].
- Update and add tests asserting the whole surface imports and serializes
without torch, and that the paths that genuinely construct torch
objects (deserializer, Triton translator) still raise a clear error
naming the extra.
Fixesaws#5531
Breaking change: users who installed plain sagemaker-serve and relied on
a torch code path (deserializing to tensor/pt, or ONNX/Triton export of a
PyTorch model) must now install sagemaker-serve[torch]. Both paths
inherently require a torch object the caller supplies, so torch is
already present in practice; a GitHub-wide search found no external
callers of these paths that do not already import torch.
---
X-AI-Prompt: address the reviewer concern on whether making torch optional breaks existing customers; find ways to reduce the torch dependency and update pyproject and docs; consolidate all changes in the goelakash93 fork for a possible major version bump.
X-AI-Tool: Claude Code
@goelakash
goelakashforce-pushed the fix-optional-torch-serve branch from 38b75f5 to 299438eCompareAugust 7, 2026 20:49
@goelakash
goelakashdeployed to manual-approval August 7, 2026 20:49 — with GitHub Actions Active
jam-jee added a commit that referenced this pull request Aug 17, 2026
The reviewer aborted on every fork PR with "Actor does not have write
permissions to the repository" (e.g. run 31217496013 on #6166), so it
only ever ran for collaborators.
claude-code-action checks that the PR author has write access before
doing anything. That default protects its normal @claude usage, where a
read-only user's comment becomes the prompt. It does not apply here:
pull_request_target always runs the base-branch copy of this workflow,
so the prompt is fixed by maintainers and a fork cannot supply it.
Set allowed_non_write_users so the review actually runs, and harden the
prompt-injection surface it exposes (untrusted diff/PR text entering
context):
- deny Read on /proc, /sys, ~/.aws, the Actions _temp dir and .git/config
so an injected instruction cannot use the review comment as a
secret-exfiltration channel
- instruct the model to treat all contributor-authored content as data,
never as instructions, and to report attempted injection
Fork PRs continue to require maintainer approval via the manual-approval
environment, Bash/Write/Edit remain unavailable, and the assumed role is
still limited to bedrock:InvokeModel on a single inference profile.
@goelakash
goelakashdeployed to manual-approval August 18, 2026 20:42 — with GitHub Actions Active
Sign up for freeto 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.

Make torch an optional dependency / extra

1 participant

@goelakash
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(serve): make torch an optional dependency - #6166

Open
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve
Open

fix(serve): make torch an optional dependency#6166
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve

Conversation

@goelakash

Copy link
Copy Markdown
Contributor

Fixes#5531

sagemaker-serve lists torch>=2.0.0 as a required dependency, so any install of sagemaker, sagemaker-serve, or sagemaker-mlops pulls torch (and on Linux, the CUDA/cuDNN/NCCL stack) even for API-only use. sagemaker-core already treats torch as an extra.

The blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in serve/constants.py, which instantiated TorchTensorSerializer() at module scope. That runs from torch import Tensor on any import sagemaker.serve. Every other torch reference in the package is already lazy.

Changes

  • Store serializer/deserializer classes in DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate at lookup in _fetch_serializer_and_deserializer_for_framework.
  • Move torch>=2.0.0 to a torch extra, matching sagemaker-core.
  • Update the two docstring examples and the affected unit tests.
  • Add tests/unit/test_optional_torch_dependency.py, mirroring the existing sagemaker-core subprocess pattern.

Installing with the torch extra is unchanged. TorchTensorSerializer() still raises the same ImportError if torch is missing when actually used.

Testing

  • tests/unit in a venv with and without torch: no new failures vs. master (53 pre-existing failures in both).
  • New tests fail on master and pass with this change.
  • Verified in a container with no torch installed: from sagemaker.serve import ModelBuilder and import sagemaker.mlops both succeed.
  • Image size for all four packages on Amazon Linux 2023: 1.45 GB -> 536 MB.

Installing sagemaker-serve (or the umbrella sagemaker, which depends on
it) pulled torch, and on Linux the ~2.9 GB CUDA/cuDNN/NCCL closure, even
for API-only use. sagemaker-core already declared torch an extra;
sagemaker-serve declared it required, so serve users paid regardless.
The import-time blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in
serve/constants.py, which instantiated TorchTensorSerializer() at module
scope, running `from torch import Tensor` on any `import sagemaker.serve`.
Changes:
- Store serializer/deserializer classes (not instances) in
DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate on lookup. Removes the
import-time torch dependency.
- Move torch>=2.0.0 to a `torch` extra in sagemaker-serve, matching
sagemaker-core. Add a `torch` extra to the umbrella sagemaker package
(sagemaker-serve[torch]) so `pip install sagemaker[torch]` works.
- Duck-type the tensor check in TorchTensorSerializer: serialization only
needs the object's own detach()/numpy(), so it no longer imports torch
at all and works whenever the caller holds a tensor.
- Make the in-process model server's torch import lazy with a CPU
fallback, so importing it no longer requires torch.
- Fix the error messages that told serve users to install the wrong
package: TorchTensorDeserializer, the Triton translator, and the ONNX
export path now name sagemaker-serve[torch].
- Update and add tests asserting the whole surface imports and serializes
without torch, and that the paths that genuinely construct torch
objects (deserializer, Triton translator) still raise a clear error
naming the extra.
Fixesaws#5531
Breaking change: users who installed plain sagemaker-serve and relied on
a torch code path (deserializing to tensor/pt, or ONNX/Triton export of a
PyTorch model) must now install sagemaker-serve[torch]. Both paths
inherently require a torch object the caller supplies, so torch is
already present in practice; a GitHub-wide search found no external
callers of these paths that do not already import torch.
---
X-AI-Prompt: address the reviewer concern on whether making torch optional breaks existing customers; find ways to reduce the torch dependency and update pyproject and docs; consolidate all changes in the goelakash93 fork for a possible major version bump.
X-AI-Tool: Claude Code
@goelakash
goelakashforce-pushed the fix-optional-torch-serve branch from 38b75f5 to 299438eCompareAugust 7, 2026 20:49
@goelakash
goelakashdeployed to manual-approval August 7, 2026 20:49 — with GitHub Actions Active
jam-jee added a commit that referenced this pull request Aug 17, 2026
The reviewer aborted on every fork PR with "Actor does not have write
permissions to the repository" (e.g. run 31217496013 on #6166), so it
only ever ran for collaborators.
claude-code-action checks that the PR author has write access before
doing anything. That default protects its normal @claude usage, where a
read-only user's comment becomes the prompt. It does not apply here:
pull_request_target always runs the base-branch copy of this workflow,
so the prompt is fixed by maintainers and a fork cannot supply it.
Set allowed_non_write_users so the review actually runs, and harden the
prompt-injection surface it exposes (untrusted diff/PR text entering
context):
- deny Read on /proc, /sys, ~/.aws, the Actions _temp dir and .git/config
so an injected instruction cannot use the review comment as a
secret-exfiltration channel
- instruct the model to treat all contributor-authored content as data,
never as instructions, and to report attempted injection
Fork PRs continue to require maintainer approval via the manual-approval
environment, Bash/Write/Edit remain unavailable, and the assumed role is
still limited to bedrock:InvokeModel on a single inference profile.
@goelakash
goelakashdeployed to manual-approval August 18, 2026 20:42 — with GitHub Actions Active
Sign up for freeto 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.

Make torch an optional dependency / extra

1 participant

@goelakash
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(serve): make torch an optional dependency - #6166

Open
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve
Open

fix(serve): make torch an optional dependency#6166
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve

Conversation

@goelakash

Copy link
Copy Markdown
Contributor

Fixes#5531

sagemaker-serve lists torch>=2.0.0 as a required dependency, so any install of sagemaker, sagemaker-serve, or sagemaker-mlops pulls torch (and on Linux, the CUDA/cuDNN/NCCL stack) even for API-only use. sagemaker-core already treats torch as an extra.

The blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in serve/constants.py, which instantiated TorchTensorSerializer() at module scope. That runs from torch import Tensor on any import sagemaker.serve. Every other torch reference in the package is already lazy.

Changes

  • Store serializer/deserializer classes in DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate at lookup in _fetch_serializer_and_deserializer_for_framework.
  • Move torch>=2.0.0 to a torch extra, matching sagemaker-core.
  • Update the two docstring examples and the affected unit tests.
  • Add tests/unit/test_optional_torch_dependency.py, mirroring the existing sagemaker-core subprocess pattern.

Installing with the torch extra is unchanged. TorchTensorSerializer() still raises the same ImportError if torch is missing when actually used.

Testing

  • tests/unit in a venv with and without torch: no new failures vs. master (53 pre-existing failures in both).
  • New tests fail on master and pass with this change.
  • Verified in a container with no torch installed: from sagemaker.serve import ModelBuilder and import sagemaker.mlops both succeed.
  • Image size for all four packages on Amazon Linux 2023: 1.45 GB -> 536 MB.

Installing sagemaker-serve (or the umbrella sagemaker, which depends on
it) pulled torch, and on Linux the ~2.9 GB CUDA/cuDNN/NCCL closure, even
for API-only use. sagemaker-core already declared torch an extra;
sagemaker-serve declared it required, so serve users paid regardless.
The import-time blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in
serve/constants.py, which instantiated TorchTensorSerializer() at module
scope, running `from torch import Tensor` on any `import sagemaker.serve`.
Changes:
- Store serializer/deserializer classes (not instances) in
DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate on lookup. Removes the
import-time torch dependency.
- Move torch>=2.0.0 to a `torch` extra in sagemaker-serve, matching
sagemaker-core. Add a `torch` extra to the umbrella sagemaker package
(sagemaker-serve[torch]) so `pip install sagemaker[torch]` works.
- Duck-type the tensor check in TorchTensorSerializer: serialization only
needs the object's own detach()/numpy(), so it no longer imports torch
at all and works whenever the caller holds a tensor.
- Make the in-process model server's torch import lazy with a CPU
fallback, so importing it no longer requires torch.
- Fix the error messages that told serve users to install the wrong
package: TorchTensorDeserializer, the Triton translator, and the ONNX
export path now name sagemaker-serve[torch].
- Update and add tests asserting the whole surface imports and serializes
without torch, and that the paths that genuinely construct torch
objects (deserializer, Triton translator) still raise a clear error
naming the extra.
Fixesaws#5531
Breaking change: users who installed plain sagemaker-serve and relied on
a torch code path (deserializing to tensor/pt, or ONNX/Triton export of a
PyTorch model) must now install sagemaker-serve[torch]. Both paths
inherently require a torch object the caller supplies, so torch is
already present in practice; a GitHub-wide search found no external
callers of these paths that do not already import torch.
---
X-AI-Prompt: address the reviewer concern on whether making torch optional breaks existing customers; find ways to reduce the torch dependency and update pyproject and docs; consolidate all changes in the goelakash93 fork for a possible major version bump.
X-AI-Tool: Claude Code
@goelakash
goelakashforce-pushed the fix-optional-torch-serve branch from 38b75f5 to 299438eCompareAugust 7, 2026 20:49
@goelakash
goelakashdeployed to manual-approval August 7, 2026 20:49 — with GitHub Actions Active
jam-jee added a commit that referenced this pull request Aug 17, 2026
The reviewer aborted on every fork PR with "Actor does not have write
permissions to the repository" (e.g. run 31217496013 on #6166), so it
only ever ran for collaborators.
claude-code-action checks that the PR author has write access before
doing anything. That default protects its normal @claude usage, where a
read-only user's comment becomes the prompt. It does not apply here:
pull_request_target always runs the base-branch copy of this workflow,
so the prompt is fixed by maintainers and a fork cannot supply it.
Set allowed_non_write_users so the review actually runs, and harden the
prompt-injection surface it exposes (untrusted diff/PR text entering
context):
- deny Read on /proc, /sys, ~/.aws, the Actions _temp dir and .git/config
so an injected instruction cannot use the review comment as a
secret-exfiltration channel
- instruct the model to treat all contributor-authored content as data,
never as instructions, and to report attempted injection
Fork PRs continue to require maintainer approval via the manual-approval
environment, Bash/Write/Edit remain unavailable, and the assumed role is
still limited to bedrock:InvokeModel on a single inference profile.
@goelakash
goelakashdeployed to manual-approval August 18, 2026 20:42 — with GitHub Actions Active
Sign up for freeto 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.

Make torch an optional dependency / extra

1 participant

@goelakash
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(serve): make torch an optional dependency - #6166

Open
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve
Open

fix(serve): make torch an optional dependency#6166
goelakash wants to merge 1 commit into
aws:masterfrom
goelakash:fix-optional-torch-serve

Conversation

@goelakash

Copy link
Copy Markdown
Contributor

Fixes#5531

sagemaker-serve lists torch>=2.0.0 as a required dependency, so any install of sagemaker, sagemaker-serve, or sagemaker-mlops pulls torch (and on Linux, the CUDA/cuDNN/NCCL stack) even for API-only use. sagemaker-core already treats torch as an extra.

The blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in serve/constants.py, which instantiated TorchTensorSerializer() at module scope. That runs from torch import Tensor on any import sagemaker.serve. Every other torch reference in the package is already lazy.

Changes

  • Store serializer/deserializer classes in DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate at lookup in _fetch_serializer_and_deserializer_for_framework.
  • Move torch>=2.0.0 to a torch extra, matching sagemaker-core.
  • Update the two docstring examples and the affected unit tests.
  • Add tests/unit/test_optional_torch_dependency.py, mirroring the existing sagemaker-core subprocess pattern.

Installing with the torch extra is unchanged. TorchTensorSerializer() still raises the same ImportError if torch is missing when actually used.

Testing

  • tests/unit in a venv with and without torch: no new failures vs. master (53 pre-existing failures in both).
  • New tests fail on master and pass with this change.
  • Verified in a container with no torch installed: from sagemaker.serve import ModelBuilder and import sagemaker.mlops both succeed.
  • Image size for all four packages on Amazon Linux 2023: 1.45 GB -> 536 MB.

Installing sagemaker-serve (or the umbrella sagemaker, which depends on
it) pulled torch, and on Linux the ~2.9 GB CUDA/cuDNN/NCCL closure, even
for API-only use. sagemaker-core already declared torch an extra;
sagemaker-serve declared it required, so serve users paid regardless.
The import-time blocker was DEFAULT_SERIALIZERS_BY_FRAMEWORK in
serve/constants.py, which instantiated TorchTensorSerializer() at module
scope, running `from torch import Tensor` on any `import sagemaker.serve`.
Changes:
- Store serializer/deserializer classes (not instances) in
DEFAULT_SERIALIZERS_BY_FRAMEWORK; instantiate on lookup. Removes the
import-time torch dependency.
- Move torch>=2.0.0 to a `torch` extra in sagemaker-serve, matching
sagemaker-core. Add a `torch` extra to the umbrella sagemaker package
(sagemaker-serve[torch]) so `pip install sagemaker[torch]` works.
- Duck-type the tensor check in TorchTensorSerializer: serialization only
needs the object's own detach()/numpy(), so it no longer imports torch
at all and works whenever the caller holds a tensor.
- Make the in-process model server's torch import lazy with a CPU
fallback, so importing it no longer requires torch.
- Fix the error messages that told serve users to install the wrong
package: TorchTensorDeserializer, the Triton translator, and the ONNX
export path now name sagemaker-serve[torch].
- Update and add tests asserting the whole surface imports and serializes
without torch, and that the paths that genuinely construct torch
objects (deserializer, Triton translator) still raise a clear error
naming the extra.
Fixesaws#5531
Breaking change: users who installed plain sagemaker-serve and relied on
a torch code path (deserializing to tensor/pt, or ONNX/Triton export of a
PyTorch model) must now install sagemaker-serve[torch]. Both paths
inherently require a torch object the caller supplies, so torch is
already present in practice; a GitHub-wide search found no external
callers of these paths that do not already import torch.
---
X-AI-Prompt: address the reviewer concern on whether making torch optional breaks existing customers; find ways to reduce the torch dependency and update pyproject and docs; consolidate all changes in the goelakash93 fork for a possible major version bump.
X-AI-Tool: Claude Code
@goelakash
goelakashforce-pushed the fix-optional-torch-serve branch from 38b75f5 to 299438eCompareAugust 7, 2026 20:49
@goelakash
goelakashdeployed to manual-approval August 7, 2026 20:49 — with GitHub Actions Active
jam-jee added a commit that referenced this pull request Aug 17, 2026
The reviewer aborted on every fork PR with "Actor does not have write
permissions to the repository" (e.g. run 31217496013 on #6166), so it
only ever ran for collaborators.
claude-code-action checks that the PR author has write access before
doing anything. That default protects its normal @claude usage, where a
read-only user's comment becomes the prompt. It does not apply here:
pull_request_target always runs the base-branch copy of this workflow,
so the prompt is fixed by maintainers and a fork cannot supply it.
Set allowed_non_write_users so the review actually runs, and harden the
prompt-injection surface it exposes (untrusted diff/PR text entering
context):
- deny Read on /proc, /sys, ~/.aws, the Actions _temp dir and .git/config
so an injected instruction cannot use the review comment as a
secret-exfiltration channel
- instruct the model to treat all contributor-authored content as data,
never as instructions, and to report attempted injection
Fork PRs continue to require maintainer approval via the manual-approval
environment, Bash/Write/Edit remain unavailable, and the assumed role is
still limited to bedrock:InvokeModel on a single inference profile.
@goelakash
goelakashdeployed to manual-approval August 18, 2026 20:42 — with GitHub Actions Active
Sign up for freeto 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.

Make torch an optional dependency / extra

1 participant

@goelakash