Skip to content

fix(utils): fall back to INFO for a blank or unknown LOG_LEVEL - #10245

Merged
tastelikefeet merged 2 commits into
modelscope:mainfrom
Lesereingrape:fix/blank-log-level-import-crash
Sep 26, 2026
Merged

tastelikefeet merged 2 commits into
modelscope:mainfrom
Lesereingrape:fix/blank-log-level-import-crash

Conversation

@Lesereingrape

Copy link
Copy Markdown
Contributor

PR type

  • Bug Fix
  • New Feature
  • Document Updates
  • More Models or Datasets Support

LOG_LEVEL is read twice in swift/utils/logger.py, and the two read sites disagree about what to do with a value that is not a level name.

  • get_logger() normalises it with getattr(logging, log_level, logging.INFO), so an unusable value falls back to INFO.
  • The module-level statement passed the raw string straight to ms_logger.setLevel(log_level), and Logger.setLevel() raises for anything that is not a level name.

So the tolerant path is the one that never gets to run:

$ LOG_LEVEL= python -c "from swift.utils import get_logger"
Traceback (most recent call last):
  ...
  File ".../swift/utils/logger.py", line 121, in <module>
    ms_logger.setLevel(log_level)
  ...
ValueError: Unknown level: ''

An empty LOG_LEVEL is easy to get by accident — ENV LOG_LEVEL= in a Dockerfile, LOG_LEVEL= in a launcher script or .env, or a variable that a scheduler still exports while empty. In that state every module that imports the logger fails, so the job dies at import time over a cosmetic setting, and the same happens for a value padded with blanks or misspelled. The documented contract is only "LOG_LEVEL: The log level, default is 'INFO'. You can set it to 'WARNING', 'ERROR', etc." — nothing promises an exception, and get_logger() already treats a bad value as INFO.

What changed

Both read sites now share one _get_log_level() helper that strips the value, upper-cases it and always returns a numeric level. Valid settings keep working exactly as before, including the spellings that already worked:

LOG_LEVEL before after
unset INFO INFO
WARNING WARNING WARNING
warning WARNING WARNING
'' ValueError at import INFO
' ' ValueError at import INFO
VERBOSE ValueError at import INFO

How it was checked

tests/utils/test_log_level.py resolves both loggers in a fresh interpreter, because the failure happens at import time and cannot be reproduced in-process. Run with the same runner CI uses:

python -m unittest tests.utils.test_log_level -v
  • without the patch: Ran 6 tests ... FAILED (failures=3) — the blank, the whitespace-padded and the unknown value, with the traceback above;
  • with the patch: Ran 6 tests ... OK, including the two controls that pin WARNING/warning to level 30 and unset to 20.

pre-commit run --files swift/utils/logger.py tests/utils/test_log_level.py passes (flake8, isort, yapf and the formatting hooks). The rest of tests/utils is unaffected: in my environment it reports errors=38, skipped=22 both with and without this patch — those errors are optional dependencies missing locally (e.g. pydantic), not this change.

No linked issue.


Disclosure: this PR was prepared, tested and submitted by an AI agent working on behalf of the account owner.

LOG_LEVEL was read twice with different tolerance: get_logger() used
getattr(logging, value, logging.INFO) while the module-level
ms_logger.setLevel(value) passed the raw string, so `LOG_LEVEL=` made
every import of swift.utils raise ValueError before a run could start.
@tastelikefeet
tastelikefeet merged commit a40e8ad into modelscope:main Sep 26, 2026
3 checks passed
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.

2 participants