Update dirfs.py: propagate transaction context to underlying filesystem
DirFileSystem now delegates its transaction and transaction_type to the wrapped filesystem, ensuring that with fs.transaction: on a dir:// instance correctly toggles the underlying file:// transaction. This restores atomic write semantics when using the directory wrapper.
Closes #1823 - the ticket also contains code to test the fix
Can we please have a test to show that this works?
Add tests for transaction context propagation in filesystem wrappers
This PR adds tests related to issue #1823 and its fix in PR #1824:
-
test_dirfs_transaction_propagation: Verifies that DirFileSystem now correctly propagates transaction context to its wrapped filesystem, ensuring files are properly deferred until commit.
-
test_cachingfs_transaction_missing_propagation: Documents that CachingFileSystem has a similar issue where transaction context is not propagated, causing immediate writes instead of deferred ones.
These tests both validate the current fix and identify areas for potential future improvements in the transaction handling system.
I updated it in dirfs.py but really it also needs to be in cached.py and seeing that its in two classes there needs to be actually a base class for implementations that wrap existing file systems.
Currently they inherit from AbstractFileSystem or AsynFileSystem but some of them declare fs in the init method and others don't...
Any class that declares fs is a wrapper class and might need to inherit from the same class or interface so that wrappers can forward to implementations
You are probably right, and we can later extract some of this common functionality for "wrapper" classes. Whether or not they delegate to the wrapped class will vary, though: write transactions for simplecache, for example, should write to local before committing, rather than depending on the remote storage's commit mechanism.