Fix recursive delete on hierarchical namespace accounts

[Tom-Newton]

Resolves: #389

Alternative fix to #453

Unfortunately #383 broke recursive delete on blob storage accounts with hierarchical namespace enabled (ADLS gen2).

azure.core.exceptions.ResourceExistsError: This operation is not permitted on a non-empty directory.
RequestId:f3042353-701e-0069-7a68-dc7b41000000
Time:2023-09-01T00:08:11.0327332Z
ErrorCode:DirectoryIsNotEmpty
Content: <?xml version="1.0" encoding="utf-8"?><Error><Code>DirectoryIsNotEmpty</Code><Message>This operation is not permitted on a non-empty directory.
RequestId:f3042353-701e-0069-7a68-dc7b41000000
Time:2023-09-01T00:08:11.0327332Z</Message></Error>

This is because #383 causes all blobs (including hierarchical namespace directory markers) do be deleted asynchronously. There is nothing to ensure that the contents of a directory are deleted before the directory itself, hence the error above.

Chosen fix

Look at the sorted list of blob names to derive which blobs are directory markers so they can be deleted in the correct order. Other blobs are deleted asynchronously. The strategy used for this is a bit strange but I think its the best option, at least for now.

Alternative solutions:

1. container_client.delete_blobs()
   1. A single call to Azure makes a batch delete. Not sure how the performance will compare to the async single deleted that we do.
   2. This looks like its works on hierarchical and flat namespaces storage accounts but it doesn't work on azurite delete_blobs method raises an exception PartialBatchErrorException when using Azurite Azure/Azurite#1809
   3. Simple solution if we have a list of blobs to delete.
2. Detect a hierarchical namespace accounts and follow a different code path
   1. Detecting hierarchical namespace is shockingly difficult to do e.g. https://github.com/apache/arrow/blob/667e9170ef363e9b4a067be3be245d2dcd4b7a6f/cpp/src/arrow/filesystem/azurefs.cc#L832-L879
   2. The delete_directory API on hierarchical namespace accounts can automatically delete the directory recursively with no need to explicitly list all the blobs it contains https://learn.microsoft.com/en-us/python/api/azure-storage-file-datalake/azure.storage.filedatalake.datalakedirectoryclient?view=azure-python#azure-storage-filedatalake-datalakedirectoryclient-delete-directory
3. Read the blob metadata to look for directory markers
   1. Directory markers on hierarchical namespace accounts have the metadata Hdi_isfolder=true.
   2. This would be a simple and robust way to detect directory marker blobs that must be deleted in the correct order.
   3. Unfortunately it will be a big performance cost to query the metadata of all these blobs unnecessarily. It seems like rm: Optimizations for multiple files. #383 added expand_path=False specifically to get this optimisation.

Testing

The currently existing test test_rm_recursive catches this bug if it is run against a hierarchical namespace storage account. I provisioned a real hierarchical namespace account to run this test against.
I added an assertion to test_rm_recursive that the directory marker is the last delete.
I also added a test that deletes more directories. To implement this I changed the storage fixture to have function scope to reset it after every test. This doesn't seem to impact the runtime of the tests so I assume this is ok.

[TomAugspurger]

cc @daavoo do you have a chance to look at this?

[Tom-Newton]

I think all the lint failures should be resolved now.

[TomAugspurger]

Looks good, thanks. Just removed a print statement in the tests. I'll merge once CI passes.

[TomAugspurger]

Thanks @Tom-Newton!

[efiop]

Thank you @Tom-Newton !

[gdippolito]

Thanks a lot! @efiop would you folks have already a date in mind to cut a release containing this patch?

[efiop]

@gdippolito Thanks for reminding! 2024.1.0 is out now.