From 16c6a0367d8f51d9991ffe7d17b9f77ebe3d25bc Mon Sep 17 00:00:00 2001 From: johnmacnamararseg <57767250+johnmacnamararseg@users.noreply.github.com> Date: Thu, 8 Feb 2024 18:09:31 -0500 Subject: [PATCH 1/4] add missing await --- adlfs/spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adlfs/spec.py b/adlfs/spec.py index 5976d185..b437092f 100644 --- a/adlfs/spec.py +++ b/adlfs/spec.py @@ -1220,7 +1220,7 @@ async def _rm_files( # marker before the directory is empty. If these are deleted out of order we will get # `This operation is not permitted on a non-empty directory.` on hierarchical namespace storage accounts. for directory_marker in reversed(directory_markers): - cc.delete_blob(directory_marker) + await cc.delete_blob(directory_marker) for file in file_paths: self.invalidate_cache(self._parent(file)) From 9980ba91e1fb1450dd77f59b0a95b7b53cccafdc Mon Sep 17 00:00:00 2001 From: "john.macnamara" Date: Mon, 12 Feb 2024 21:50:20 -0500 Subject: [PATCH 2/4] split tests, one to test full remove, one to test callable order --- adlfs/tests/test_spec.py | 44 +++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/adlfs/tests/test_spec.py b/adlfs/tests/test_spec.py index 6aa06120..76c9aa0a 100644 --- a/adlfs/tests/test_spec.py +++ b/adlfs/tests/test_spec.py @@ -2,7 +2,6 @@ import tempfile from unittest import mock -import azure.storage.blob import azure.storage.blob.aio import dask.dataframe as dd import numpy as np @@ -754,14 +753,8 @@ def test_rm_recursive(mock_delete_blob, storage): mock.ANY, "root/c" ), "The directory deletion should be the last call" - -@mock.patch.object( - azure.storage.blob.aio.ContainerClient, - "delete_blob", - side_effect=azure.storage.blob.aio.ContainerClient.delete_blob, - autospec=True, -) -def test_rm_recursive2(mock_delete_blob, storage): +@pytest.mark.filterwarnings("error") +def test_rm_recursive2(storage): fs = AzureBlobFileSystem( account_name=storage.account_name, connection_string=CONN_STR ) @@ -769,12 +762,42 @@ def test_rm_recursive2(mock_delete_blob, storage): assert "data/root" in fs.ls("/data") fs.rm("data/root", recursive=True) + assert "data/root" not in fs.ls("/data") with pytest.raises(FileNotFoundError): fs.ls("data/root") - last_deleted_paths = [call.args[1] for call in mock_delete_blob.mock_calls[-7:]] + +async def test_rm_recursive_call_order(storage, mocker): + from azure.storage.blob.aio import ContainerClient + + delete_blob_mock = mocker.patch.object(ContainerClient, "delete_blob", return_value=None) + fs = AzureBlobFileSystem( + account_name=storage.account_name, connection_string=CONN_STR + ) + + container_name = "data" + file_paths = [ + "root/a", + "root/a/file.txt", + "root/a1", + "root/a1/file1.txt", + "root/b", + "root/b/file.txt", + "root", + "root/c", + "root/c/file1.txt", + "root/c/file2.txt", + "root/d", + "root/d/file_with_metadata.txt", + "root/e+f", + "root/e+f/file1.txt", + "root/e+f/file2.txt", + "root/rfile.txt" + ] + await fs._rm_files(container_name=container_name, file_paths=file_paths) + last_deleted_paths = [call.args[0] for call in delete_blob_mock.call_args_list[-7:]] assert last_deleted_paths == [ "root/e+f", "root/d", @@ -786,6 +809,7 @@ def test_rm_recursive2(mock_delete_blob, storage): ], "The directory deletion should be in reverse lexographical order" + def test_rm_multiple_items(storage): fs = AzureBlobFileSystem( account_name=storage.account_name, connection_string=CONN_STR From 7cf1d9ec5903945d02f6c3c3a2bb3b089086fc65 Mon Sep 17 00:00:00 2001 From: "john.macnamara" Date: Mon, 12 Feb 2024 22:05:00 -0500 Subject: [PATCH 3/4] remove empty line --- adlfs/tests/test_spec.py | 1 - 1 file changed, 1 deletion(-) diff --git a/adlfs/tests/test_spec.py b/adlfs/tests/test_spec.py index 76c9aa0a..4f990fe8 100644 --- a/adlfs/tests/test_spec.py +++ b/adlfs/tests/test_spec.py @@ -809,7 +809,6 @@ async def test_rm_recursive_call_order(storage, mocker): ], "The directory deletion should be in reverse lexographical order" - def test_rm_multiple_items(storage): fs = AzureBlobFileSystem( account_name=storage.account_name, connection_string=CONN_STR From 4d23ead7222b4ba91af747014ba8b90e8c106cd7 Mon Sep 17 00:00:00 2001 From: "john.macnamara" Date: Mon, 12 Feb 2024 22:09:04 -0500 Subject: [PATCH 4/4] remove empty line --- adlfs/tests/test_spec.py | 1 - 1 file changed, 1 deletion(-) diff --git a/adlfs/tests/test_spec.py b/adlfs/tests/test_spec.py index 4f990fe8..aa39ed4f 100644 --- a/adlfs/tests/test_spec.py +++ b/adlfs/tests/test_spec.py @@ -762,7 +762,6 @@ def test_rm_recursive2(storage): assert "data/root" in fs.ls("/data") fs.rm("data/root", recursive=True) - assert "data/root" not in fs.ls("/data") with pytest.raises(FileNotFoundError):