fix: run dynamo components standalone (1/2) #1560
Conversation
mohammedabdulwahhab
changed the title
…lwahhab/sdk-refactor
mohammedabdulwahhab
changed the title
mohammedabdulwahhab
requested review from
nnshah1,
rmccorm4,
whoisj,
a team,
hutm,
biswapanda,
ishandhanani,
julienmancuso and
hhzhang16
as code owners
| return getattr(instance, name) | ||
| return None | ||
|
|
||
| def get_ep_handlers(endpoints, inner_instance): |
There was a problem hiding this comment.
Can we call this get_endpoint_handlers. EP is very overloaded these days :)
|
|
||
| # 6. Finally serve each endpoint | ||
| handlers = get_ep_handlers(drt_endpoints, inner_instance) | ||
| ep_2_handler = {endpoints[ep_name]: handlers[ep_name] for ep_name in endpoints} |
There was a problem hiding this comment.
what is this ep_2_handler?
There was a problem hiding this comment.
mapping between endpoint (from drt) to the actual request handler that should be used to serve it
endpoint = component.endpoint("foo")
handler = # actual function to serve the endpoint with
... # below
endpoint.serve_endpoint(handler)
|
Does this PR address |
WalkthroughThe changes introduce a standalone serving mechanism for Dynamo-based services, including a new CLI module, context model, and serving coroutine. Example files demonstrate usage patterns for SDK v2, including context injection and endpoint streaming. Additionally, a markdown document outlines personas and API design proposals for serving components with varying levels of control. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Script
participant Serve as serve()
participant Service as Service Class
participant Runtime as Dynamo Runtime
participant Endpoint as Endpoint Handler
User->>Serve: Call serve(Service)
Serve->>Runtime: Initialize or use runtime
Serve->>Service: Instantiate (inject DynamoContext if needed)
alt Service defines async_init
Serve->>Service: Await async_init()
end
Serve->>Endpoint: Bind endpoint handlers to Service methods
Serve->>Runtime: Register endpoint handlers
Serve->>Endpoint: Start serving endpoints (async)
Poem
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: 5
🧹 Nitpick comments (8)
examples/sdk_v2/mixed.py (2)
25-25: Remove unused import.
uvloopis imported but not used.- import uvloop
1-38: Apply black formatting.The code needs to be formatted according to the project's style guide.
Run
black examples/sdk_v2/mixed.pyto fix formatting issues.examples/sdk_v2/default.py (2)
77-77: Remove unused import.
uvloopis imported but not used.- import uvloop
1-79: Apply black formatting.The code needs to be formatted according to the project's style guide.
Run
black examples/sdk_v2/default.pyto fix formatting issues.deploy/sdk/src/dynamo/sdk/cli/serve_standalone.py (2)
3-3: Remove unused imports.
Listandconfigure_dynamo_loggingare imported but not used.-from typing import List, Any, Dict +from typing import Any, Dict-from dynamo.runtime.logging import configure_dynamo_loggingAlso applies to: 9-9
1-106: Apply black formatting.The code needs to be formatted according to the project's style guide.
Run
black deploy/sdk/src/dynamo/sdk/cli/serve_standalone.pyto fix formatting issues.examples/sdk_v2/v2.md (2)
93-93: Fix trailing whitespace.Remove trailing whitespace at the end of line 93.
-``` +```
97-98: Fix markdown formatting issues.
- Fix list indentation (should be 2 spaces, not 4)
- Remove trailing punctuation from heading
- giving user access to the runtime, component, endpoints without a global dynamo_context dict - - user can introspect serve() function to see how this injection is happening - - extremely clear when dynamo_context is populated, since it is being injected + - user can introspect serve() function to see how this injection is happening + - extremely clear when dynamo_context is populated, since it is being injected-### Persona 3: Power user: Total control over how the endpoints are served and don't want anything to happen automatically. +### Persona 3: Power user: Total control over how the endpoints are served and don't want anything to happen automaticallyAlso applies to: 107-107
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
deploy/sdk/src/dynamo/sdk/__init__.py(1 hunks)deploy/sdk/src/dynamo/sdk/cli/serve_standalone.py(1 hunks)examples/sdk_v2/default.py(1 hunks)examples/sdk_v2/mixed.py(1 hunks)examples/sdk_v2/v2.md(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
deploy/sdk/src/dynamo/sdk/__init__.py
26-26: dynamo.sdk.cli.serve_standalone.serve imported but unused
(F401)
examples/sdk_v2/mixed.py
3-3: Undefined name service
(F821)
12-12: Undefined name async_init
(F821)
17-17: Undefined name endpoint
(F821)
18-18: Undefined name ChatRequest
(F821)
25-25: uvloop imported but unused
Remove unused import: uvloop
(F401)
28-28: Undefined name serve
(F821)
deploy/sdk/src/dynamo/sdk/cli/serve_standalone.py
3-3: typing.List imported but unused
Remove unused import: typing.List
(F401)
9-9: Module level import not at top of file
(E402)
9-9: dynamo.runtime.logging.configure_dynamo_logging imported but unused
Remove unused import: dynamo.runtime.logging.configure_dynamo_logging
(F401)
10-10: Module level import not at top of file
(E402)
11-11: Module level import not at top of file
(E402)
12-12: Module level import not at top of file
(E402)
examples/sdk_v2/default.py
77-77: uvloop imported but unused
Remove unused import: uvloop
(F401)
🪛 GitHub Actions: Pre Merge Validation of (ai-dynamo/dynamo/refs/pull/1560/merge) by mohammedabdulwahhab.
deploy/sdk/src/dynamo/sdk/__init__.py
[error] 21-45: ESLint: dynamo.sdk.cli.serve_standalone.serve imported but unused; consider removing, adding to __all__, or using a redundant alias (F401)
[error] isort formatting check failed. Files were modified to fix import order issues.
examples/sdk_v2/mixed.py
[error] 4-4: ESLint: Undefined name service (F821)
[error] 13-13: ESLint: Undefined name async_init (F821)
[error] 23-23: ESLint: Undefined name endpoint (F821)
[error] 24-24: ESLint: Undefined name ChatRequest (F821)
[error] 35-35: ESLint: Undefined name serve (F821)
[error] black formatting check failed. Files were reformatted to comply with code style.
deploy/sdk/src/dynamo/sdk/cli/serve_standalone.py
[error] 12-14: ESLint: Module level import not at top of file (E402)
[error] black formatting check failed. Files were reformatted to comply with code style.
examples/sdk_v2/default.py
[error] black formatting check failed. Files were reformatted to comply with code style.
examples/sdk_v2/v2.md
[error] trailing-whitespace check failed. Files were modified to remove trailing whitespace.
🪛 Pylint (3.3.7)
examples/sdk_v2/mixed.py
[error] 3-3: Undefined variable 'service'
(E0602)
[error] 15-15: Instance of 'Router' has no 'namespace' member
(E1101)
[error] 17-17: Undefined variable 'endpoint'
(E0602)
[error] 18-18: Undefined variable 'ChatRequest'
(E0602)
[error] 28-28: Undefined variable 'serve'
(E0602)
deploy/sdk/src/dynamo/sdk/cli/serve_standalone.py
[refactor] 20-20: Too few public methods (0/2)
(R0903)
[refactor] 18-18: Too few public methods (0/2)
(R0903)
[error] 76-76: No value for argument 'runtime' in function call
(E1120)
examples/sdk_v2/default.py
[refactor] 21-21: Too few public methods (0/2)
(R0903)
[refactor] 25-25: Too few public methods (0/2)
(R0903)
🪛 GitHub Actions: Copyright Checks
examples/sdk_v2/mixed.py
[error] 1-1: Invalid or missing copyright header detected. Please add the required SPDX copyright and license header.
deploy/sdk/src/dynamo/sdk/cli/serve_standalone.py
[error] 1-1: Invalid or missing copyright header detected. Please add the required SPDX copyright and license header.
🪛 LanguageTool
examples/sdk_v2/v2.md
[uncategorized] ~13-~13: Possible missing comma found.
Context: ...me cases ## Proposal In the following section I'll iterate through variations that sh...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~49-~49: Try using a synonym here to strengthen your writing.
Context: ...at do we need to add next? - Ability to give access to the runtime, endpoints, compo...
(GIVE_PROVIDE)
[style] ~98-~98: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ...e how this injection is happening - extremely clear when dynamo_context is populated, since...
(EN_WEAK_ADJECTIVE)
[uncategorized] ~101-~101: Did you mean “my”?
Context: ...rve() is still serving my endpoints for me - serve() creates a DynamoContext insta...
(FOR_ME_FOR_MY)
[uncategorized] ~105-~105: Possible missing comma found.
Context: ...hat are facilitating the serving of the component and not have that hidden behind a serve...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~109-~109: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ything to happen automatically. Wants top level code to use raw bindings to serve compo...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~109-~109: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...level code to use raw bindings to serve component. Might also want to pass args and kwarg...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~109-~109: You might be missing the article “the” here.
Context: ... also want to pass args and kwargs from top level to the constructor ```python fro...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
examples/sdk_v2/v2.md
97-97: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
98-98: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
107-107: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test - vllm
🔇 Additional comments (2)
examples/sdk_v2/default.py (1)
46-46: Update or clarify the comment aboutdynamo serve.The comment states this block is run with
dynamo serve, but the PR objective is to enable standalone execution withpython3 -m. This seems contradictory.Should this comment be updated to reflect the new standalone serving capability? The main block already uses
serve(Backend)for standalone execution.deploy/sdk/src/dynamo/sdk/cli/serve_standalone.py (1)
37-38: ```shell
#!/bin/bashDisplay the serve function including inner worker and its invocation
sed -n '1,200p' deploy/sdk/src/dynamo/sdk/cli/serve_standalone.py
Locate where
workeris called within the filerg -n "await worker" deploy/sdk/src/dynamo/sdk/cli/serve_standalone.py
rg -n "worker(" deploy/sdk/src/dynamo/sdk/cli/serve_standalone.py</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
mohammedabdulwahhab
requested review from
grahamking,
kkranen and
tedzhouhk
as code owners
When running components using python3, they don't need any depends statements. Is this what is being asked? |
| """ | ||
| sig = inspect.signature(service.inner.__init__) | ||
| params = list(sig.parameters.keys()) | ||
| return len(params) > 1 and params[1] == "dynamo_context" |
There was a problem hiding this comment.
can we introspect type along with variable name - warn if there is a type mismatch.
There was a problem hiding this comment.
yep, agreed. I think that's a fix worth keeping
…amo/dynamo into mabdulwahhab/sdk-refactor
| parser.add_argument("--name", default="User", help="Name to use (for client)") | ||
| args = parser.parse_args() | ||
|
|
||
| uvloop.install() |
There was a problem hiding this comment.
Should this not be happenign in serve_standalone?
Overview:
Allows components to be run directly with python3 while being inter-operable with dynamo serve
docs: https://github.com/ai-dynamo/dynamo/blob/9e839039cb7ae882c047092bad1da87394f873c9/docs/API/sdk.md#direct-python-invocation
Testing
closes https://linear.app/nvidia-dynamo/issue/DEP-173/issue-user-should-be-able-to-launch-a-component-using-python3-m
Summary by CodeRabbit
New Features
Documentation