LocalFileHelper.NewWriter should create directories if necessary #23
Conversation
This file contains 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
Owner
jlewi
commented
Sep 18, 2024
jlewi
commented
- If the directories don't exist the NewWriter should create it.
- I don't think it makes sense to make it the callers responsibility because not all filesystems (e.g. object storage) have a notion of directories so its better to make it the FileHelper's responsibility.
* If the directories don't exist the NewWriter should create it. * I don't think it makes sense to make it the callers responsibility because not all filesystems (e.g. object storage) have a notion of directories so its better to make it the FileHelper's responsibility.
jlewi
added a commit
to jlewi/foyle
that referenced
this pull request
Sep 18, 2024
* The bug is described in #215 (comment) * The bug is that `Config.getTrainingDirs` returns no training directories if config.learner == nil * Prior to #211 config.learner would be non-nil because we had to set the path of the RunMe logs * However, now that we no longer depend on RunMe logs config.learner could be nil and this would return no training directories. In which case learner.Reconcile would not attempt to save any examples * The fix is to allow config.Learner to be nil in GetTrainingDirs and return a suitable default. We also need to ensure the training directory gets created if it doesn't exist. * This is fixed by jlewi/monogo#23 which updates LocalFileHelper to create the directory if it doesn't exist. * My suspicion is that I never hit this bug because i originally created my ~/.foyle/training directory using a version of the code which wasn't using FileHelper and explicitly checked and created the directory. I suspect when I refactored the code to support saving examples to GCS thats when the code to ensure the directory exists got dropped.
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.