Skip to content

fix: [#102] Fix configuration priority order to match documentation#391

Open
nandanosql wants to merge 1 commit into
bytedance:mainfrom
nandanosql:fix/issue-102-config-priority
Open

fix: [#102] Fix configuration priority order to match documentation#391
nandanosql wants to merge 1 commit into
bytedance:mainfrom
nandanosql:fix/issue-102-config-priority

Conversation

@nandanosql
Copy link
Copy Markdown

Fixes #102

Problem

The resolve_config_value function in config.py had conflicting priority definitions:

  • Code: CLI > ENV > Config > Default
  • Documentation (README): CLI > Config > ENV > Default

This caused confusion and unexpected behavior where environment variables would override values in the config file, contrary to what users would expect from reading the docs.

Solution

Reordered the priority checks in resolve_config_value to match the documented priority:

# Before (wrong):
if cli_value is not None: return cli_value
if env_var and os.getenv(env_var): return os.getenv(env_var)  # ENV checked before Config
if config_value is not None: return config_value

# After (correct):
if cli_value is not None: return cli_value
if config_value is not None: return config_value  # Config checked before ENV
if env_var and os.getenv(env_var): return os.getenv(env_var)

The docstring was also updated to reflect the correct priority.

…ntation

The resolve_config_value function was checking ENV before Config,
but the README documents the priority as:
  CLI > Configuration file > Environment variables > Default values

This commit reorders the checks to match the documented priority:
  CLI > Config > ENV > Default
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: The conflict of the definition of "configuration priority" between readme.md and ~/utils/config.py

2 participants