From dff1c167c0ce67e83b1df005940673e6818a2706 Mon Sep 17 00:00:00 2001 From: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:13:04 +0100 Subject: [PATCH] fix(python-helpers): honor environment configuration precedence Fixes #2102. Add regression and compatibility coverage. Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> --- .../nv-cloud-function-helpers/AGENTS.md | 10 +- .../nvcf_container/helpers.py | 7 +- .../tests/test_config_value.py | 97 +++++++++++++++++++ 3 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 src/libraries/python/nv-cloud-function-helpers/tests/test_config_value.py diff --git a/src/libraries/python/nv-cloud-function-helpers/AGENTS.md b/src/libraries/python/nv-cloud-function-helpers/AGENTS.md index 1adb67e8e2..190be2ea3b 100644 --- a/src/libraries/python/nv-cloud-function-helpers/AGENTS.md +++ b/src/libraries/python/nv-cloud-function-helpers/AGENTS.md @@ -32,5 +32,11 @@ Declared in `setup.py`: ## Tests -No test suite is present yet. Add pytest coverage alongside changes when -modifying helpers. +Run the helper tests from this directory with pytest installed: + +```sh +python3 -m pytest -q tests +``` + +Add regression coverage alongside helper changes. Tests use local fixtures and +do not need a running Triton or NVCF service. diff --git a/src/libraries/python/nv-cloud-function-helpers/nv_cloud_function_helpers/nvcf_container/helpers.py b/src/libraries/python/nv-cloud-function-helpers/nv_cloud_function_helpers/nvcf_container/helpers.py index f52aac8b8c..f7cf3e5bc1 100644 --- a/src/libraries/python/nv-cloud-function-helpers/nv_cloud_function_helpers/nvcf_container/helpers.py +++ b/src/libraries/python/nv-cloud-function-helpers/nv_cloud_function_helpers/nvcf_container/helpers.py @@ -169,12 +169,9 @@ def get_config_value(value_name: str, model_config: dict = None) -> str: """ returns a value from Triton's model config or from environment variable with the priority given to the environment """ - if model_config is None: + if model_config is None or value_name in os.environ: return os.environ[value_name] - else: - return os.environ.get( - value_name, model_config["parameters"][value_name]["string_value"] - ) + return model_config["parameters"][value_name]["string_value"] def load_image(input_str: str, root_dir: str, has_transparency: bool = False): diff --git a/src/libraries/python/nv-cloud-function-helpers/tests/test_config_value.py b/src/libraries/python/nv-cloud-function-helpers/tests/test_config_value.py new file mode 100644 index 0000000000..0b0d4b7a54 --- /dev/null +++ b/src/libraries/python/nv-cloud-function-helpers/tests/test_config_value.py @@ -0,0 +1,97 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 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. + +"""Environment settings take precedence without evaluating unused fallbacks.""" + +from copy import deepcopy + +import pytest + +from nv_cloud_function_helpers.nvcf_container.helpers import get_config_value + +KEY = "NVCF_TEST_CONFIG_OVERRIDE" + + +@pytest.mark.parametrize("environment_value", ["configured", ""]) +@pytest.mark.parametrize( + "config", + [ + None, + {}, + {"parameters": {}}, + {"parameters": {KEY: {}}}, + {"parameters": {KEY: {"string_value": "fallback"}}}, + ], +) +def test_environment_wins_even_without_model_fallback( + monkeypatch, environment_value, config +): + monkeypatch.setenv(KEY, environment_value) + before = deepcopy(config) + assert get_config_value(KEY, config) == environment_value + assert config == before + + +@pytest.mark.parametrize("model_value", ["fallback", "", "0"]) +def test_model_value_is_used_when_environment_is_absent( + monkeypatch, model_value +): + monkeypatch.delenv(KEY, raising=False) + config = {"parameters": {KEY: {"string_value": model_value}}} + before = deepcopy(config) + assert get_config_value(KEY, config) == model_value + assert config == before + + +@pytest.mark.parametrize( + "config, missing_key", + [ + (None, KEY), + ({}, "parameters"), + ({"parameters": {}}, KEY), + ({"parameters": {KEY: {}}}, "string_value"), + ], +) +def test_missing_values_keep_their_key_errors(monkeypatch, config, missing_key): + monkeypatch.delenv(KEY, raising=False) + with pytest.raises(KeyError) as error: + get_config_value(KEY, config) + assert error.value.args == (missing_key,) + + +def test_unused_fallback_is_not_read(monkeypatch): + class UnreadableConfig(dict): + def __getitem__(self, key): + raise AssertionError("Environment override must not read fallback") + + monkeypatch.setenv(KEY, "override") + assert get_config_value(KEY, UnreadableConfig()) == "override" + + +def test_environment_changes_are_not_cached(monkeypatch): + config = {"parameters": {KEY: {"string_value": "fallback"}}} + monkeypatch.setenv(KEY, "first") + assert get_config_value(KEY, config) == "first" + monkeypatch.setenv(KEY, "second") + assert get_config_value(KEY, config) == "second" + monkeypatch.delenv(KEY) + assert get_config_value(KEY, config) == "fallback" + + +def test_unrelated_environment_value_does_not_override(monkeypatch): + monkeypatch.delenv(KEY, raising=False) + monkeypatch.setenv(KEY + "_OTHER", "different") + config = {"parameters": {KEY: {"string_value": "fallback"}}} + assert get_config_value(KEY, config) == "fallback"