Repository navigation
feat: Extract Common Configs + Log Configs on Init + Add test_ to sdk/tests filenames required for pytest
#434
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.
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
Merged
ishandhanani
merged 51 commits into
ai-dynamo:main
from
jon-chuang:jon-chuang/extract-common-config
Apr 9, 2025
Merged
Changes from 40 commits
Commits
Show all changes
51 commits
Select commit
Hold shift + click to select a range
e9602f0
done
jon-chuang 1dd9ed7
fix
jon-chuang b65cb3d
improve
jon-chuang 1266f5b
improve
jon-chuang 664dde5
improve
jon-chuang 8b8f56b
improve
jon-chuang bf923e8
improve
jon-chuang c043d14
update
jon-chuang e65b243
mypy
jon-chuang 311ea5d
minor
jon-chuang 2a5befb
log
jon-chuang 070f78c
z
jon-chuang afccf7b
oops
jon-chuang 8925e41
done
jon-chuang 4ea445b
minor
jon-chuang 0973a76
empty
jon-chuang 5c124ca
logger.info
jon-chuang b000d39
Merge branch 'main' of https://github.com/ai-dynamo/dynamo into jon-c…
jon-chuang 7be91d1
Merge branch 'main' into jon-chuang/extract-common-config
ishandhanani c4d9895
improve
jon-chuang 8948fc8
Merge branch 'jon-chuang/extract-common-config' of https://github.com…
jon-chuang 05630f1
fix
jon-chuang f86d2bd
fix
jon-chuang ec82dd5
Merge branch 'main' into jon-chuang/extract-common-config
jon-chuang 41957ec
Merge branch 'main' into jon-chuang/extract-common-config
jon-chuang 3dde003
Merge branch 'main' of https://github.com/ai-dynamo/dynamo into jon-c…
jon-chuang e5d7449
Merge branch 'jon-chuang/extract-common-config' of https://github.com…
jon-chuang 6b49f62
add tests
jon-chuang 76e9570
minor
jon-chuang 9b4539c
oops
jon-chuang 4fd1a83
done
jon-chuang 513c486
done
jon-chuang 83ae44e
done
jon-chuang d3af2ea
stricter
jon-chuang b8996f4
fix
jon-chuang d984167
Merge branch 'main' of https://github.com/ai-dynamo/dynamo into jon-c…
jon-chuang 85d2e95
add backwards compat
jon-chuang 765bb4b
simplify tests
jon-chuang 5d24017
done
jon-chuang ef64586
Merge branch 'main' into jon-chuang/extract-common-config
ishandhanani fa0f1b6
mark pre-merge
jon-chuang 702fd0c
Merge branch 'jon-chuang/extract-common-config' of https://github.com…
jon-chuang 3c90836
oops
jon-chuang 4eb8fea
rename tests so that pytest picks them up
jon-chuang 2d06e3f
improve doc comment
jon-chuang 25fc1ad
oops
jon-chuang f06e9e6
fix test
jon-chuang 7cb53f0
try this
jon-chuang 1ce1c81
hmm
jon-chuang bc6f8f6
relax
jon-chuang 6ef0750
hmm
jon-chuang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| import os | ||
|
|
||
| from dynamo.sdk.lib.config import ServiceConfig | ||
|
|
||
|
|
||
| def test_service_config_with_common_configs(): | ||
| # Reset singleton instance | ||
| ServiceConfig._instance = None | ||
|
|
||
| # Set environment variable with config that includes common-configs | ||
| os.environ[ | ||
| "DYNAMO_SERVICE_CONFIG" | ||
| ] = """ | ||
| { | ||
| "Common": { | ||
| "model": "deepseek-ai/DeepSeek-R1-Distill-Llama-8B", | ||
| "block-size": 64, | ||
| "max-model-len": 16384 | ||
| }, | ||
| "VllmWorker": { | ||
| "enforce-eager": true, | ||
| "common-configs": ["model", "block-size", "max-model-len"] | ||
| } | ||
| } | ||
| """ | ||
|
|
||
| # Get arguments and verify common configs are included | ||
| service_config = ServiceConfig.get_instance() | ||
| vllm_worker_args = service_config.as_args("VllmWorker") | ||
|
|
||
| # Check that each common config appears in the arguments | ||
| for key in ["model", "block-size", "max-model-len"]: | ||
| assert f"--{key}" in vllm_worker_args | ||
|
|
||
|
|
||
| def test_service_config_without_common_configs(): | ||
| # Reset singleton instance | ||
| ServiceConfig._instance = None | ||
|
|
||
| # Set environment variable with config that DOESN'T include common-configs | ||
| os.environ[ | ||
| "DYNAMO_SERVICE_CONFIG" | ||
| ] = """ | ||
| { | ||
| "Common": { | ||
| "model": "deepseek-ai/DeepSeek-R1-Distill-Llama-8B", | ||
| "block-size": 64, | ||
| "max-model-len": 16384 | ||
| }, | ||
| "VllmWorker": { | ||
| "enforce-eager": true | ||
| } | ||
| } | ||
| """ | ||
|
|
||
| # Get arguments and verify common configs are NOT included | ||
| service_config = ServiceConfig.get_instance() | ||
| vllm_worker_args = service_config.as_args("VllmWorker") | ||
|
|
||
| # Check that none of the common configs appear in arguments | ||
| for key in ["model", "block-size", "max-model-len"]: | ||
| assert f"--{key}" not in vllm_worker_args | ||
|
|
||
|
|
||
| def test_service_config_with_direct_configs(): | ||
| # Reset singleton instance | ||
| ServiceConfig._instance = None | ||
|
|
||
| # Set environment variable with direct configs (no Common section reference) | ||
| os.environ[ | ||
| "DYNAMO_SERVICE_CONFIG" | ||
| ] = """ | ||
| { | ||
| "VllmWorker": { | ||
| "enforce-eager": true, | ||
| "model": "deepseek-ai/DeepSeek-R1-Distill-Llama-8B", | ||
| "block-size": 64, | ||
| "max-model-len": 16384 | ||
| } | ||
| } | ||
| """ | ||
|
|
||
| # Get arguments and verify direct configs are included | ||
| service_config = ServiceConfig.get_instance() | ||
| vllm_worker_args = service_config.as_args("VllmWorker") | ||
|
|
||
| # Check that each config appears in the arguments | ||
| for key in ["model", "block-size", "max-model-len"]: | ||
| assert f"--{key}" in vllm_worker_args | ||
|
|
||
|
|
||
| def test_service_config_override_common_configs(): | ||
| # Reset singleton instance | ||
| ServiceConfig._instance = None | ||
|
|
||
| # Set environment variable with config that includes common-configs | ||
| # overridden by the subscribing config | ||
| os.environ[ | ||
| "DYNAMO_SERVICE_CONFIG" | ||
| ] = """ | ||
| { | ||
| "Common": { | ||
| "model": "deepseek-ai/DeepSeek-R1-Distill-Llama-8B", | ||
| "block-size": 64, | ||
| "max-model-len": 16384 | ||
| }, | ||
| "VllmWorker": { | ||
| "enforce-eager": true, | ||
| "block-size": 128, | ||
| "common-configs": ["model", "block-size", "max-model-len"] | ||
| } | ||
| } | ||
| """ | ||
|
|
||
| # Get arguments and verify common configs are included | ||
| service_config = ServiceConfig.get_instance() | ||
| vllm_worker_args = service_config.as_args("VllmWorker") | ||
|
|
||
| # Check that each common config appears in the arguments | ||
| for key in ["model", "block-size", "max-model-len"]: | ||
| assert f"--{key}" in vllm_worker_args | ||
|
|
||
| assert vllm_worker_args[vllm_worker_args.index("--block-size") + 1] == 128 | ||
|
nnshah1 marked this conversation as resolved.
Resolved
|
||
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
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.