fix: create parent directories for nested NOTEWISE_HOME config path#120
fix: create parent directories for nested NOTEWISE_HOME config path#120algojogacor wants to merge 1 commit into
Conversation
Change config_dir.mkdir(exist_ok=True) to mkdir(parents=True, exist_ok=True) in get_config_path() so setup works when NOTEWISE_HOME points to a nested path whose parents do not exist yet. This matches the parent-safe creation already used in repository.py, logging.py, and _documents.py. Fixes whoisjayd#109
|
@algojogacor is attempting to deploy a commit to the whoisjayd Team on Vercel. A member of the Team first needs to authorize it. |
Reviewer's GuideEnsures get_config_path creates parent directories for NOTEWISE_HOME and adds tests covering nested and pre-existing NOTEWISE_HOME scenarios. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the get_config_path function to use parents=True when creating the configuration directory, ensuring that nested paths are correctly initialized. Corresponding unit tests were added to verify this behavior. The reviewer noted that the current implementation of get_config_path introduces side effects by creating directories, suggesting that this logic should ideally be moved to a dedicated initialization step or save_config to avoid unintended filesystem modifications during read-only operations.
| """Get path to user config file.""" | ||
| config_dir = get_state_dir() | ||
| config_dir.mkdir(exist_ok=True) | ||
| config_dir.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
The get_config_path function has a side effect of creating the configuration directory on the filesystem. This is generally discouraged for a 'getter' function, especially since it is called by read-only operations like load_config and show_current_config. This can lead to unexpected directory creation even when just checking for a configuration's existence, and it can cause side effects during unit testing (e.g., creating ~/.notewise if the path is not explicitly mocked or redirected). Additionally, it results in redundant mkdir calls when load_config is used within save_config. Consider moving the mkdir logic to save_config or a dedicated initialization step.
|
closing since it is automated |
Summary
Fixes #109
When
NOTEWISE_HOMEpoints to a nested path whose parents do not exist,notewise setupfails before writing config becauseget_config_path()only callsmkdir(exist_ok=True)withoutparents=True.Root Cause
get_config_path()insrc/notewise/ui/setup_wizard.py:78creates the state directory withconfig_dir.mkdir(exist_ok=True), which raisesFileNotFoundErrorwhen intermediate parent directories are missing. Other state/output code paths (repository.py, logging.py, _documents.py) already useparents=True, creating an inconsistency.Fix
Changed
config_dir.mkdir(exist_ok=True)toconfig_dir.mkdir(parents=True, exist_ok=True), matching the existing parent-safe creation pattern used by:src/notewise/storage/repository.py:40src/notewise/logging.py:256src/notewise/pipeline/_documents.py:119Changes
src/notewise/ui/setup_wizard.py: 1 line changed (+1 -1)tests/unit/ui/test_setup_wizard.py: 2 test cases added (+25 -0)Testing
Summary by Sourcery
Ensure setup wizard creates configuration directories safely when NOTEWISE_HOME points to nested paths.
Bug Fixes:
Tests:
Summary by CodeRabbit
Bug Fixes
Tests