8000 fix: serviceArgs in config was not getting set for workers by mohammedabdulwahhab · Pull Request #640 · ai-dynamo/dynamo · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: serviceArgs in config was not getting set for workers #640

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Apr 11, 2025
Merged

fix: serviceArgs in config was not getting set for workers #640

merged 4 commits into from
Apr 11, 2025

Conversation

mohammedabdulwahhab
Copy link
Contributor
@mohammedabdulwahhab mohammedabdulwahhab commented Apr 11, 2025 •
edited
Loading

Overview:

ServiceArgs portion of DYNAMO_SERVICE_CONFIG will always be empty from the POV of workers due to this line. This MR fixes this issue.

Then why does setting gpus or workers in the config file currently work (e.g here)? Workers and gpus are attributes consumed and processed by the top level program. They aren't consumed at all in the subprocesses. At the top level program these attributes are available and okay. However, any attributes that need to be consumed at the subprocess level will simply not be present.

Testing

Tested:

  • hello world
  • Aggregated Serving
  • Disaggregated Serving

@mohammedabdulwahhab mohammedabdulwahhab added bug Something isn't working 8000 dynamo-serve Relates to dynamo-serve labels Apr 11, 2025
@mohammedabdulwahhab mohammedabdulwahhab self-assigned this Apr 11, 2025
@copy-pr-bot copy-pr-bot
Copy link
copy-pr-bot bot commented Apr 11, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

hhzhang16
hhzhang16 reviewed Apr 11, 2025
View reviewed changes
@mohammedabdulwahhab mohammedabdulwahhab enabled auto-merge (squash) April 11, 2025 22:54
@mohammedabdulwahhab mohammedabdulwahhab merged commit e3857ba into main Apr 11, 2025
7 checks passed
@mohammedabdulwahhab mohammedabdulwahhab deleted the mabdulwahhab/fix-overrides branch April 11, 2025 23:15
oandreeva-nv pushed a commit that referenced this pull request Apr 22, 2025
@mohammedabdulwahhab @oandreeva-nv
fix: serviceArgs in config was not getting set for workers (#640)
fcd7109
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hhzhang16 hhzhang16 hhzhang16 approved these changes

@hutm hutm Awai 8639 ting requested review from hutm hutm is a code owner

@biswapanda biswapanda Awaiting requested review from biswapanda biswapanda is a code owner

@ishandhanani ishandhanani Awaiting requested review from ishandhanani ishandhanani is a code owner

@julienmancuso julienmancuso Awaiting requested review from julienmancuso julienmancuso is a code owner

@nnshah1 nnshah1 Awaiting requested review from nnshah1 nnshah1 is a code owner

Assignees

@mohammedabdulwahhab mohammedabdulwahhab

Labels
bug Something isn't working dynamo-serve Relates to dynamo-serve
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mohammedabdulwahhab @hhzhang16
0