squirrel-core
squirrel-core copied to clipboard
Safety checks for store and driver using FilePathGenerator
Description
For both store and driver we need to asses if a URL points to an empty directory or nested empty directories.
- For drivers, warning the user when using empty directories alerts the user early on that the url might be invalid
- For stores, we want to only overwrite an existing non-empty directory when it is explicitly allowed
In both cases, checking if the directories/nested directories are empty are done through the FilePathGenerator
Type of change
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Documentation update
- [ ] Refactoring including code style reformatting
- [ ] Other (please describe):
Checklist:
- [ ] I have read the contributing guideline doc (external contributors only)
- [ ] Lint and unit tests pass locally with my changes
- [ ] I have kept the PR small so that it can be easily reviewed
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] All dependency changes have been reflected in the pip requirement files.
When we create a driver, a store is automatically created. Here, we want to have exist_ok=True
, but when we use the store in the driver to write shards, we can still have duplicate data. How should it be handled, wdyt @AlpAribal @AlirezaSohofi ?
When we create a driver, a store is automatically created. Here, we want to have
exist_ok=True
, but when we use the store in the driver to write shards, we can still have duplicate data. How should it be handled, wdyt @AlpAribal @AlirezaSohofi ?
How about having a read_only
property in store? We can set this to False by default, and StoreDriver sets it to True. We can raise when user tries to write with a Driver's store, prompt them to manually set store.read_only=False
This is PR is marked as stale as it has been inactive for 30 days. It will be closed in 7 days.