feat: deprecate sdk as dependency #2149
Conversation
biswapanda
requested review from
a team,
alec-flowers,
hhzhang16,
hutm,
ishandhanani,
julienmancuso,
mohammedabdulwahhab,
nnshah1,
ptarasiewiczNV,
richardhuo-nv,
rmccorm4 and
tanmayv25
as code owners
biswapanda
changed the title
WalkthroughThis change removes the entire Dynamo SDK Python package, including all CLI tools, core abstractions, resource management, protocol interfaces, decorators, deployment logic, tests, and related documentation. It also updates build scripts, Dockerfiles, and configuration files to eliminate references to the deleted SDK, CLI, and associated binaries. Changes
Sequence Diagram(s)No sequence diagrams generated as the changes are purely deletions and configuration cleanups. Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.gitignore (1)
87-92: Deduplicate.build/ignore rule
.build/already appears at line 87; the re-addition at line 92 is redundant noise in version control rules.-.build/container/Dockerfile.vllm (1)
288-292: Avoid self-appending$PYTHONPATHtwiceThe new line expands to
ENV PYTHONPATH=:…:, inserting the current value twice when the variable is already empty at image-build time.
A simpler, idempotent form keeps the intent while avoiding duplicate separators.-ENV PYTHONPATH=$PYTHONPATH:$HOME/dynamo/components/planner/src:$PYTHONPATH +ENV PYTHONPATH=$HOME/dynamo/components/planner/src:$PYTHONPATH
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
.devcontainer/post-create.sh(0 hunks).gitignore(1 hunks)Earthfile(0 hunks)container/Dockerfile.sglang(2 hunks)container/Dockerfile.sglang-wideep(0 hunks)container/Dockerfile.tensorrt_llm(0 hunks)container/Dockerfile.vllm(1 hunks)deploy/CONTRIBUTING.md(0 hunks)deploy/sdk/README.md(0 hunks)deploy/sdk/docs/cli/README.md(0 hunks)deploy/sdk/docs/sdk/README.md(0 hunks)deploy/sdk/src/dynamo/sdk/__init__.py(0 hunks)deploy/sdk/src/dynamo/sdk/cli/Dockerfile.template(0 hunks)deploy/sdk/src/dynamo/sdk/cli/allocator.py(0 hunks)deploy/sdk/src/dynamo/sdk/cli/circus.py(0 hunks)deploy/sdk/src/dynamo/sdk/cli/cli.py(0 hunks)deploy/sdk/src/dynamo/sdk/cli/env.py(0 hunks)deploy/sdk/src/dynamo/sdk/cli/run.py(0 hunks)deploy/sdk/src/dynamo/sdk/cli/run_executable.py(0 hunks)deploy/sdk/src/dynamo/sdk/cli/serve.py(0 hunks)deploy/sdk/src/dynamo/sdk/cli/serve_dynamo.py(0 hunks)deploy/sdk/src/dynamo/sdk/cli/serving.py(0 hunks)deploy/sdk/src/dynamo/sdk/cli/utils.py(0 hunks)deploy/sdk/src/dynamo/sdk/core/__init__.py(0 hunks)deploy/sdk/src/dynamo/sdk/core/decorators/endpoint.py(0 hunks)deploy/sdk/src/dynamo/sdk/core/lib.py(0 hunks)deploy/sdk/src/dynamo/sdk/core/protocol/__init__.py(0 hunks)deploy/sdk/src/dynamo/sdk/core/protocol/deployment.py(0 hunks)deploy/sdk/src/dynamo/sdk/core/protocol/interface.py(0 hunks)deploy/sdk/src/dynamo/sdk/core/runner/__init__.py(0 hunks)deploy/sdk/src/dynamo/sdk/core/runner/common.py(0 hunks)deploy/sdk/src/dynamo/sdk/core/runner/dynamo.py(0 hunks)deploy/sdk/src/dynamo/sdk/core/runner/health.py(0 hunks)deploy/sdk/src/dynamo/sdk/lib/__init__.py(0 hunks)deploy/sdk/src/dynamo/sdk/lib/config.py(0 hunks)deploy/sdk/src/dynamo/sdk/lib/decorators.py(0 hunks)deploy/sdk/src/dynamo/sdk/lib/exceptions.py(0 hunks)deploy/sdk/src/dynamo/sdk/lib/image.py(0 hunks)deploy/sdk/src/dynamo/sdk/lib/loader.py(0 hunks)deploy/sdk/src/dynamo/sdk/lib/resource.py(0 hunks)deploy/sdk/src/dynamo/sdk/lib/utils.py(0 hunks)deploy/sdk/src/dynamo/sdk/tests/config.yaml(0 hunks)deploy/sdk/src/dynamo/sdk/tests/pipeline.py(0 hunks)deploy/sdk/src/dynamo/sdk/tests/test_config.py(0 hunks)deploy/sdk/src/dynamo/sdk/tests/test_e2e_args.py(0 hunks)deploy/sdk/src/dynamo/sdk/tests/test_e2e_config.py(0 hunks)deploy/sdk/src/dynamo/sdk/tests/test_link.py(0 hunks)deploy/sdk/src/dynamo/sdk/tests/test_resources.py(0 hunks)deploy/sdk/tests/test_deployment.sh(0 hunks)pyproject.toml(0 hunks)
💤 Files with no reviewable changes (47)
- deploy/CONTRIBUTING.md
- deploy/sdk/src/dynamo/sdk/core/init.py
- deploy/sdk/docs/sdk/README.md
- deploy/sdk/docs/cli/README.md
- pyproject.toml
- container/Dockerfile.sglang-wideep
- deploy/sdk/src/dynamo/sdk/core/protocol/init.py
- container/Dockerfile.tensorrt_llm
- deploy/sdk/src/dynamo/sdk/lib/image.py
- deploy/sdk/src/dynamo/sdk/lib/init.py
- deploy/sdk/README.md
- deploy/sdk/src/dynamo/sdk/cli/Dockerfile.template
- deploy/sdk/src/dynamo/sdk/cli/run.py
- deploy/sdk/src/dynamo/sdk/tests/test_resources.py
- deploy/sdk/src/dynamo/sdk/tests/test_link.py
- deploy/sdk/src/dynamo/sdk/tests/test_e2e_args.py
- deploy/sdk/src/dynamo/sdk/cli/cli.py
- deploy/sdk/src/dynamo/sdk/lib/exceptions.py
- deploy/sdk/src/dynamo/sdk/cli/run_executable.py
- deploy/sdk/src/dynamo/sdk/lib/decorators.py
- deploy/sdk/src/dynamo/sdk/tests/test_e2e_config.py
- deploy/sdk/src/dynamo/sdk/cli/allocator.py
- deploy/sdk/src/dynamo/sdk/core/protocol/deployment.py
- deploy/sdk/src/dynamo/sdk/cli/env.py
- deploy/sdk/src/dynamo/sdk/core/decorators/endpoint.py
- deploy/sdk/src/dynamo/sdk/core/runner/health.py
- deploy/sdk/src/dynamo/sdk/tests/pipeline.py
- deploy/sdk/src/dynamo/sdk/tests/config.yaml
- deploy/sdk/src/dynamo/sdk/cli/serving.py
- .devcontainer/post-create.sh
- deploy/sdk/src/dynamo/sdk/lib/loader.py
- deploy/sdk/src/dynamo/sdk/core/runner/init.py
- deploy/sdk/tests/test_deployment.sh
- deploy/sdk/src/dynamo/sdk/cli/circus.py
- Earthfile
- deploy/sdk/src/dynamo/sdk/cli/serve_dynamo.py
- deploy/sdk/src/dynamo/sdk/lib/utils.py
- deploy/sdk/src/dynamo/sdk/core/runner/common.py
- deploy/sdk/src/dynamo/sdk/cli/utils.py
- deploy/sdk/src/dynamo/sdk/core/protocol/interface.py
- deploy/sdk/src/dynamo/sdk/lib/config.py
- deploy/sdk/src/dynamo/sdk/cli/serve.py
- deploy/sdk/src/dynamo/sdk/tests/test_config.py
- deploy/sdk/src/dynamo/sdk/init.py
- deploy/sdk/src/dynamo/sdk/lib/resource.py
- deploy/sdk/src/dynamo/sdk/core/runner/dynamo.py
- deploy/sdk/src/dynamo/sdk/core/lib.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: biswapanda
PR: ai-dynamo/dynamo#1412
File: lib/bindings/python/src/dynamo/runtime/logging.py:100-100
Timestamp: 2025-06-06T21:48:35.214Z
Learning: In the Dynamo codebase, BentoML has been completely removed from all executable code, with only documentation and attribution references remaining. The error_loggers configuration in lib/bindings/python/src/dynamo/runtime/logging.py should not include "bentoml" since those modules no longer exist.
container/Dockerfile.vllm (1)
Learnt from: grahamking
PR: #1177
File: container/Dockerfile.vllm:102-105
Timestamp: 2025-05-28T22:54:46.875Z
Learning: In Dockerfiles, when appending to environment variables that may not exist in the base image, Docker validation will fail if you reference undefined variables with ${VARIABLE} syntax. In such cases, setting the environment variable directly (e.g., ENV CPATH=/usr/include) rather than appending is the appropriate approach.
container/Dockerfile.sglang (2)
Learnt from: nnshah1
PR: #1444
File: tests/fault_tolerance/scenarios.py:57-57
Timestamp: 2025-07-01T15:39:56.789Z
Learning: The fault tolerance tests in tests/fault_tolerance/ are designed to run only in the mounted container environment, so hardcoded absolute paths with /workspace/ prefix are intentional and should not be changed to relative paths.
Learnt from: julienmancuso
PR: #1474
File: deploy/cloud/operator/internal/controller/dynamocomponent_controller.go:1302-1306
Timestamp: 2025-06-11T21:18:00.425Z
Learning: In the Dynamo operator, the project’s preferred security posture is to set a Pod-level PodSecurityContext with runAsUser, runAsGroup, and fsGroup all set to 1000, and then selectively override the user at the individual container level (e.g., RunAsUser: 0 for Kaniko) when root is required.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Test - vllm
biswapanda
requested review from
GuanLuo,
PeaBrane,
grahamking,
jthomson04,
kkranen,
paulhendricks,
piotrm-nvidia,
ryanolson,
tedzhouhk and
tmonty12
as code owners
hhzhang16
left a comment
hhzhang16
left a comment
There was a problem hiding this comment.
Approved with respect to planner work
|
Awesome! |
Overview:
deprecate sdk as dependency based
Why?
Dynamo-UX v2 simplifies deploy and launch process by completely removing dynamo sdk based pythonic layer and commands.
What changed -
Closes: DYN-720
Details:
Where should the reviewer start?
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
Bug Fixes
Chores
.gitignoreand build configuration files to remove SDK-specific entries.