fix: ensure config directory exists before mkstemp in _write_config_snapshot - #9856
Open
xiaoyuyu6420 wants to merge 1 commit into
Open
fix: ensure config directory exists before mkstemp in _write_config_snapshot#9856xiaoyuyu6420 wants to merge 1 commit into
xiaoyuyu6420 wants to merge 1 commit into
Conversation
…napshot AstrBotConfig._write_config_snapshot calls tempfile.mkstemp(dir=directory) without verifying the directory exists. When a config profile is created for the first time (e.g. create_conf instantiates AstrBotConfig with a brand-new path), __init__ calls save_config before the directory is guaranteed to exist, and mkstemp raises FileNotFoundError. Add os.makedirs(directory, exist_ok=True) before mkstemp. This is the standard defensive pattern for atomic-write helpers. Fixes three intermittently-failing dashboard tests: - test_t2i_set_active_template_syncs_all_configs - test_t2i_reset_default_template_syncs_all_configs - test_t2i_update_active_template_reloads_all_schedulers Closes AstrBotDevs#9855.
Contributor
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. If the path is wrong or directory creation has unintended side effects, merging can leave an extra directory on disk after the code is reverted. That state is bounded and can be removed manually; the normal failure mode is a config snapshot write error rather than irreversible data loss.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Author
|
Friendly ping @Soulter — this is a 4-line fix ( |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
Three dashboard tests are intermittently failing on
master(different ones fail on different runs):test_t2i_set_active_template_syncs_all_configstest_t2i_reset_default_template_syncs_all_configstest_t2i_update_active_template_reloads_all_schedulersThe traceback points to
tempfile.mkstemp()insideAstrBotConfig._write_config_snapshot()(astrbot/core/config/astrbot_config.py:290).Root Cause
_write_config_snapshotcreates a temp file in the config file's parent directory, but never ensures that directory exists:When
AstrBotConfig.__init__(line 64-67) detects the config file doesn't exist yet, it callssave_config()→_write_config_snapshot→mkstemp(dir=directory). If the directory hasn't been created (fresh profile creation, test fixture race),mkstempraisesFileNotFoundError.The
create_confpath inastrbot_config_mgr.py:208-210triggers this — it constructs a newconf_pathand instantiatesAstrBotConfig(config_path=conf_path, ...), which callssave_config()during__init__before the directory is guaranteed to exist.Fix
Add
os.makedirs(directory, exist_ok=True)before themkstempcall. One line, standard defensive pattern for atomic-write helpers.Verification
Before fix (current
masterd2d7e5a):After fix (5 consecutive runs):
Full suite: 2201 passed, 0 failed.
ruff format/ruff checkclean.Closes #9855.
Summary by Sourcery
Bug Fixes: