refactor: refactor dynamo serve part-1/N #788
Merged
Conversation
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
31173c2 to
c01e8fe
Compare
cd5ba88 to
40911e8
Compare
nnshah1
reviewed
Apr 25, 2025
rmccorm4
reviewed
Apr 30, 2025
Comment on lines
+53
to
+56
| dynamo={ | ||
| "enabled": True, | ||
| "namespace": "dynamo", | ||
| }, |
There was a problem hiding this comment.
@biswapanda was this a newly required change for Frontend to work with these refactor changes?
There was a problem hiding this comment.
- On trtllm example, it seems like http and llmctl commands are no longer getting run -- I noticed you didn't add the dynamo service section in examples/tensorrt_llm/components/frontend.py
- After I added this to try to fix it locally, I'm getting errors now:
2025-04-30T01:10:50.985Z INFO serve_dynamo.worker: [Frontend:1] Registering component dynamo/Frontend
2025-04-30T01:10:50.985Z INFO serve_dynamo.worker: [Frontend:1] Created Frontend component
2025-04-30T01:10:50.986Z ERROR serve_dynamo.worker: [Frontend:1] FATAL ERROR: No Dynamo endpoints found in service Frontend!
2025-04-30T01:10:50.986Z ERROR serve_dynamo.worker: [Frontend:1] Error in Dynamo component setup: FATAL ERROR: No Dynamo endpoints found in service Frontend!
2025-04-30T01:10:50.986Z ERROR serve_dynamo: [Frontend:1] Error in main: FATAL ERROR: No Dynamo endpoints found in service Frontend!
Seems like these changes broke the usage of http+llmctl as frontend if no dynamo endpoint is defined in it?
This was referenced Apr 30, 2025
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Overview:
This PR is part 1/N to refactor dynamo serve path and introduce loose coupling with circusd
httpserver in llm example.credits: related PR #721 from @ishandhanani
Where should the reviewer start?
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
https://linear.app/nvidia-dynamo/issue/DYN-185