fix(python-helpers): honor environment configuration precedence - #2103
sylvesterkaczmarek wants to merge 1 commit into
Conversation
Fixes NVIDIA#2102. Add regression and compatibility coverage. Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA/nvcf/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. 📝 WalkthroughWalkthrough
ChangesConfiguration override behavior
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to The environment override change preserves the checked missing-value behavior. No issue identified here needs resolution before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
TL;DR
Honor environment-variable precedence in the Python container helper without evaluating an unused model-config fallback. Valid environment overrides currently raise KeyError when the corresponding model-config fields are absent.
Additional Details
os.environ.get(name, fallback)evaluates the fallback before checking the environment. Inget_config_value, that fallback indexes the nested Triton model configuration, so an environment value alone cannot supply a parameter when a partial model config is passed.Read an existing environment value first, including an explicitly empty string. Only consult
model_configwhen the environment value is absent. Preserve the existing fallback lookup and KeyError details when neither source provides a value. The function signature and return types are unchanged.This is a local Python helper correction. It adds no request handling, logging of values, network calls, telemetry, or service configuration. Runtime dependencies and NOTICE files are unchanged. The subtree's test instructions now describe the added pytest suite.
For the Reviewer
The production change is confined to
get_config_valueinsrc/libraries/python/nv-cloud-function-helpers/nv_cloud_function_helpers/nvcf_container/helpers.py. The new tests are in that package'stests/test_config_value.py.All open PR file lists were checked for overlap; none modified this helper subtree. Issue #2102 was opened on 25 September 2026 and rechecked as unassigned with no other claimant or competing fix before submission. The patch is based on main
3414c033.For QA
From
src/libraries/python/nv-cloud-function-helpers, with the package and pytest installed:git diff --checkpass. The commit is signed off and its signature is verified by GitHub.Validation is scoped to this standalone helper package, following its local test guidance. The wider NVCF monorepo, Bazel service targets, live Triton containers, and deployed NVCF were not tested. The package's existing missing-README source-distribution warning remains. No real credentials were read and no service or cluster was started or modified. Service-level integration remains for maintainer QA.
Issues
Fixes #2102.
Checklist
Summary by CodeRabbit
Bug Fixes
KeyErrorinstead of returning a default.Documentation