-
Notifications
You must be signed in to change notification settings - Fork 110
Set rm_files to be a synchronous method #503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
78d8ba9
b62bb25
c54a64a
b8d09f3
cab1c15
915295c
ef84f97
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2210,3 +2210,38 @@ def test_write_max_concurrency(storage, max_concurrency, blob_size, blocksize): | |
with fs.open(path, "rb") as f: | ||
assert f.read() == data | ||
fs.rm(container_name, recursive=True) | ||
|
||
|
||
def test_rm_file(storage): | ||
fs = AzureBlobFileSystem( | ||
account_name=storage.account_name, | ||
connection_string=CONN_STR, | ||
) | ||
path = "data/test_file.txt" | ||
with fs.open(path, "wb") as f: | ||
f.write(b"test content") | ||
|
||
assert fs.exists(path) | ||
fs.rm_file(path) | ||
with pytest.raises(FileNotFoundError): | ||
fs.ls(path) | ||
assert not fs.exists(path) | ||
assert path not in fs.dircache | ||
|
||
|
||
def test_rm_file_versioned_blob(storage): | ||
fs = AzureBlobFileSystem( | ||
account_name=storage.account_name, | ||
connection_string=CONN_STR, | ||
version_aware=True, | ||
) | ||
path = "data/test_file.txt?versionid=latest" | ||
with fs.open(path, "wb") as f: | ||
f.write(b"test content") | ||
|
||
|
||
assert fs.exists(path) | ||
|
||
fs.rm_file(path) | ||
with pytest.raises(FileNotFoundError): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's also add another separate test case where we try to call |
||
fs.ls(path) | ||
assert not fs.exists(path) | ||
assert path not in fs.dircache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poking more through the adlfs codebase, I'm thinking we will also need to invalidate the
path
as well. While it does not seem like some of the other methods handle this (and arguably something we should circle back to addressing eventually), we can run into inconsistencies where the path can still reside in the dircache. For example, ifls()
is run on the file, it still shows up as present:We should make sure to add a test case for this as well.