Directories and delimiter handling

[hayesgb]

Opened based on #554 .

Azure Storage consists of a 2-level hierarch, where the top level is a container, and all blobs underneath a container have a flat structure. Delimiters ("/") that denote folders in traditional filesystems have no meaning. Azure implements a BlobPrefix object for convenience, but the BlobPrefix can not be created directly, and is immediately removed when all blobs underneath the prefix are deleted.

This creates challenges for filesystem operations like mkdir(), because the making of an empty directory can only be done by creating an empty blob. The result is that:

fs.mkdir("container/blob_folder")
fs.mkdir("container/blob_folder/")

both appear as size=0 blobs, and are unique. Choosing the former convention creates issues as described here with partitioned parquet files, while the latter approach runs counter to the convention of removing a trailing "/" when listing directories and/or storing them in dircache, as evidenced by #554 .

Its my understanding this is an issue with s3fs and gcsfs. Do these filesystems exhibit similar challenges, and if so, how are they handled there? Hoping to align on an approach that provides consistency for users, and with the use of dircache.

Thanks,
Greg

[martindurant]

Thanks for opening this, and let's take the chance to nail down how we would like this to work.
Yes, s3fs and gcsfs face the exact same structure (buckets/keys and prefix/delimiter emulation) and problems. I'll take a moment to describe the current general conventions we have.

In most cases, paths are stripped of any trailing "/". mkdir is a no-op for anything below buckets, and directories are only "created" by containing other keys. In comments, these are usually called "pseudo-directories".

However, since the appearance of recursive operations and one-shot find, there is a certain amount of awkward code to back-fill inferred directories. There are known fail cases where paths end with "/", which would be illegal on posix; but they do appear when using the web consoles and some frameworks (spark?). Note that in dircache stores listings keyed by their (pseudo-)directory, and that these keys don't end in "/", but that means we don't know which listing to put a name ending "/" into.

So I suggest:

• a set of short tests showing what ls, glob and find should return in various situations with or without "/"-ended path names, and at various depths including the bucket level (NB: a bucket name cannot contain"/")
• for the same set of circumstances, explicitly checking that dircache contains the expected entries; i.e., repeated calling of ls/find/glob should still return the right output.

[martindurant]

I should add, that mkdir bein a no-op means that you get conterintuitive behaviour sometimes

>>> s3.mkdir("bucket/path")
>>> s3.exists("bucket/path")
False

but I don't think there's a way around this; the even more unsatisfactory situation (involving more calls) still fails intuition in the opposite direction

>>> s3.exists("bucket/path")
False
>>> s3.touch("bucket/path/file")  # OK
>>> s3.isdir("bucket/path")
True

[hayesgb]

Originally, adlfs implemented fs.mkdir() by creating a blob (rather than a no-op) without the trailing delimiter and size=0. This eventually led to empty blobs being created by pyarrow to_dataset() operations. The current verison in master will create an empty blob with a trailing delimiter, and this has resolved the problem of empty blobs, but means that:

    ## these are containers
    assert fs.ls("") == ["data/"]
    assert fs.ls("/") == ["data/"]
    assert fs.ls(".") == ["data/"]

    ## these are top-level directories and files
    assert fs.ls("data") == ["data/root/", "data/top_file.txt"]
    assert fs.ls("/data") == ["data/root/", "data/top_file.txt"]

    # root contains files and directories
    assert fs.ls("data/root") == [
        "data/root/a/",
        "data/root/a1/",
        "data/root/b/",
        "data/root/c/",
        "data/root/d/",
        "data/root/rfile.txt",
    ]

It should resolve fs.exists(), because an object does exist in that location. It can create confounding situations, where fs.isdir() or fs.isfile() return incorrect results. However, Azure blobs provide a metadata parameter, and with the mkdir or AzureBlobFile objects, I've been looking at adding the following: metadata['is_directory': "true or false" when an object is written.

It sounds as if your preference would be to omit the trailing delimiter from a pseudo-directory and leave mkdir as a no-op. Is that correct?

[martindurant]

It sounds as if your preference would be to omit the trailing delimiter from a pseudo-directory and leave mkdir as a no-op. Is that correct?

It seems to me to be the intent of the API; the advantage being avoiding the need for extra calls to make the objects. I like the metadata idea, but presumably we'd also have to account for trees that were not written with this convention.

[hayesgb]

Thinking about �mkdir() being a no-op, from above:

>>> s3.mkdir("bucket/path")
>>> s3.exists("bucket/path")

This also means that:

>>> s3.ls("bucket") = []
>>> s3.touch("bucket/path/file") <-- This creates a BlobPrefix for path
>>> s3.ls("bucket") = ["path"]     <-- Now the pseudo-directory appears
>>> s3.rm("bucket/path/file")      <-- Remove the file, which automatically deletes the BlobPrefix
>>> s3.ls("bucket") = []

correct?

This means s3.rmdir() should also be a no-op, for anything except buckets/containers, right?

Agree the reduction in calls is an advantage of the API, but are users OK with this behavior?

[martindurant]

Yes, that's exactly what currently happens for s3fs. I can't remember, rmdir might give a helpful "directory not empty" message.
In practice, I think people do rm(path, recursive=True), which may attempt to also delete the non-existent pseudo-directory and ignore the failure.

[hayesgb]

Tagging @isidentical , @ldacey, and @jorisvandenbossche

Hoping for input from some users of adlfs that have given input on its development on the above discussion.

[isidentical]

Support for empty directories is really important for our use case. Not as in creating them with .mkdir() but rather being able to do fs.isdir('empty/dir/') and fs.ls('empty/dir/') so that when someone creates a directory using the UI of that provider (for example "Create Directory" option in google cloud) we could understand it and create a local directory. Regarding .mkdir(), I have no comments.

[hayesgb]

I've been testing the current behavior in Azure. If I go to the portal, and create:

adlfs/test_folder/test_inner

Then I create a container_client, and call container_client.walk_blobs(name_starts_with="test_folder"), I get back a generator that, when printed, returns:

<class 'azure.storage.blob._list_blobs_helper.BlobPrefix'>
{'name': 'testfolder/', 'prefix': 'testfolder/', 'results_per_page': None, 'container': 'adlfs', 'delimiter': None, 'location_mode': 'primary'}
<class 'azure.storage.blob._list_blobs_helper.BlobPrefix'>
{'name': 'testfolder/test_inner/', 'prefix': 'testfolder/test_inner/', 'results_per_page': None, 'container': 'adlfs', 'delimiter': None, 'location_mode': 'primary'}

But if I call container_client.list_blobs(name_starts_with="test_folder"), I get back an iterator that returns:

<class 'azure.storage.blob._models.BlobProperties'>
{'name': 'testfolder', 'container': 'adlfs', 'snapshot': None, 'version_id': None, 'is_current_version': None, 'blob_type': <BlobType.BlockBlob: 'BlockBlob'>, 'metadata': {}, 'encrypted_metadata': None, 'last_modified': datetime.datetime(2021, 3, 10, 20, 30, 3, tzinfo=datetime.timezone.utc), 'etag': '0x8D8E403491339A9', 'size': 0, 'content_range': None, 'append_blob_committed_block_count': None, 'is_append_blob_sealed': None, 'page_blob_sequence_number': None, 'server_encrypted': True, 'copy': {'id': None, 'source': None, 'status': None, 'progress': None, 'completion_time': None, 'status_description': None, 'incremental_copy': None, 'destination_snapshot': None}, 'content_settings': {'content_type': 'application/octet-stream', 'content_encoding': None, 'content_language': None, 'content_md5': None, 'content_disposition': None, 'cache_control': 'no-cache'}, 'lease': {'status': 'unlocked', 'state': 'available', 'duration': None}, 'blob_tier': 'Hot', 'rehydrate_priority': None, 'blob_tier_change_time': None, 'blob_tier_inferred': True, 'deleted': None, 'deleted_time': None, 'remaining_retention_days': None, 'creation_time': datetime.datetime(2021, 3, 10, 20, 30, 3, tzinfo=datetime.timezone.utc), 'archive_status': None, 'encryption_key_sha256': None, 'encryption_scope': None, 'request_server_encrypted': None, 'object_replication_source_properties': [], 'object_replication_destination_policy': None, 'last_accessed_on': None, 'tag_count': None, 'tags': None}

The list_blobs() response is consistent with a "pseudo-folder" in Azure Blob being an empty blob.

Uploading a blob using either the client, or the portal to: testfolder2/test_inner2/myfile.txt, and subsequently deleting testfolder2/test_inner2/myfile.txt removes only myfile.txt� and leaves the nested folders in place.

This is not consistent with s3 described above. Azure SDK performs the following:

cc.delete_blob(blob="testfolder2/test_inner2/myfile.rtf")
cc.list_blobs(name_starts_with="testfolder2/test_inner2")  <-- This returns a blob of size=0 with name="testfolder2/test_inner2"

So leaving mkdir as a no-op would work, for writes, but the delete would leave the "pseudo-folder" blobs in place.

[martindurant]

Uploading a blob using either the client, or the portal to: testfolder2/test_inner2/myfile.txt, and subsequently deleting testfolder2/test_inner2/myfile.txt removes only myfile.txt� and leaves the nested folders in place.

This would not be true if the placeholder "adlfs/test_folder/test_inner" had not been previously created though, right?

[hayesgb]

Yes, it is true. See the following, using the Azure Storage Blob Python SDK directly:

>>> sc = BlobServiceClient(account_url = <my_url>, credential = <token>)
>>> cc = sc.get_container_client('my-container')
>>> cc.create_container()     <-- Creates a new container
>>> cc.upload_blob(name="testfolder/test_inner/myfile.rtf", data=b"012346789")  # Create a nested blob.

>>> blobs = cc.list_blobs(name_starts_with="testfolder/test_inner")
>>> for blob in blobs:
>>>     print(blob.name)

testfolder/test_inner
testfolder/test_inner/myfile.rtf

>>> cc.delete_blob(blob = "testfolder/test_inner/myfile.rtf")
>>> blobs2 = cc.list_blobs(name_starts_with="testfolder/test_inner")
>>> for blob in blobs2:
>>>     print(blob)

{'name': 'testfolder/test_inner', 'container': 'my-container', 'snapshot': None, 'version_id': None, 'is_current_version': None, 'blob_type': <BlobType.BlockBlob: 'BlockBlob'>, 'metadata': {}, 'encrypted_metadata': None, 'last_modified': datetime.datetime(2021, 3, 12, 3, 19, 4, tzinfo=datetime.timezone.utc), 'etag': '0x8D8E505973EE89A', 'size': 0, 'content_range': None, 'append_blob_committed_block_count': None, 'is_append_blob_sealed': None, 'page_blob_sequence_number': None, 'server_encrypted': True, 'copy': {'id': None, 'source': None, 'status': None, 'progress': None, 'completion_time': None, 'status_description': None, 'incremental_copy': None, 'destination_snapshot': None}, 'content_settings': {'content_type': None, 'content_encoding': None, 'content_language': None, 'content_md5': None, 'content_disposition': None, 'cache_control': None}, 'lease': {'status': 'unlocked', 'state': 'available', 'duration': None}, 'blob_tier': 'Hot', 'rehydrate_priority': None, 'blob_tier_change_time': None, 'blob_tier_inferred': True, 'deleted': None, 'deleted_time': None, 'remaining_retention_days': None, 'creation_time': datetime.datetime(2021, 3, 12, 3, 19, 4, tzinfo=datetime.timezone.utc), 'archive_status': None, 'encryption_key_sha256': None, 'encryption_scope': None, 'request_server_encrypted': None, 'object_replication_source_properties': [], 'object_replication_destination_policy': None, 'last_accessed_on': None, 'tag_count': None, 'tags': None}

Notice that the pseudo-directory remains in place, and it's a BlockBlob with size = 0.

Both of the following operations return a StorageError "Specified blob already exists"

cc.upload_blob(name = "testfolder/test_inner/", data="")
cc.upload_blob(name = "testfolder/test_inner", data="")

[hayesgb]

I'm on board with mkdir being a no-op when the bucket exists. This means we would expect the following behavior, if bucket does not exist:

>>> fs.mkdir("bucket")  # Creates bucket
>>> fs.mkdir("bucket/dir/nested_dir", create_parents=False)  # Raises a PermissionError
>>> fs.mkdir("bucket/dir/nested_dir", create_parents=True)   # I would expect this to create bucket, but none of the inner pseudo-directories.  Thoughts?

[hayesgb]

Additionally, once bucket is created:

>>> assert "bucket" in fs.ls(".")
>>> assert "bucket/" not in fs.ls(".")

And the following should be true, when bucket does not exist:

fs.makedir("bucket", exist_ok=False) <-- Raises a FileExistsError
fs.makedir("bucket", exist_ok=True) <-- Passes silently