Skip to content
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

rm does not delete the directory if its input is a directory with recursive True #918

Open
LuchiLucs opened this issue Dec 2, 2024 · 5 comments

Comments

@LuchiLucs
Copy link

As stated in the title, it seems to me, that the API rm does not remove the directory itself if the input path its a directory and the recursive flag is set to True:

  await self.fs._rm(
      path=f"{self.bucket_name}/{remote_path}",
      recursive=True,
      **kwargs,
  )

I checked by s3 bucket from the dashboard and the directory does still exists, while its content does not. The documentation says that:

recursive (bool) – If file(s) are directories, recursively delete contents and then also remove the directory

I guess the implementation does not follow the documentation and a direct rmdir API call is needed in order to delete also the directory.

@LuchiLucs
Copy link
Author

In the debugger I saw that:

    async def _rm(self, path, recursive=False, **kwargs):
        if recursive and isinstance(path, str):
            bucket, key, _ = self.split_path(path)
            if not key and await self._is_bucket_versioned(bucket):
                # special path to completely remove versioned bucket
                await self._rm_versioned_bucket_contents(bucket)
        paths = await self._expand_path(path, recursive=recursive)
        files = [p for p in paths if self.split_path(p)[1]]
        dirs = [p for p in paths if not self.split_path(p)[1]]

In this code, all the paths (real dirs and files) are detected in the files variable hwile the dirs variable is an empty list

@martindurant
Copy link
Member

It seems expand_path is not recovering the zero-length directory placeholder with "/" suffix. We could arbitarily add it here, since attempting to remove it if it doesn't exist shouldn't have an cost, or fix expand_path. I think I prefer the former: "path/" isn't really a member of "path".

@LuchiLucs
Copy link
Author

LuchiLucs commented Dec 4, 2024

If I try a direct call to the empy dir:

await self.fs._rmdir(
   path=f"{self.bucket_name}/{remote_path}",
)

There is the exception "FileExistsError". The path is empty though.

@martindurant
Copy link
Member

It is

await self.fs._rm_file(
   path=f"{self.bucket_name}/{remote_path}/",
)

(with the "/" suffix) that you want. This is a FILE, which is exactly the problem.

@LuchiLucs
Copy link
Author

It is

await self.fs._rm_file(
   path=f"{self.bucket_name}/{remote_path}/",
)

(with the "/" suffix) that you want. This is a FILE, which is exactly the problem.

I tried without success. If I use as value for the path either a "file path" as .../remote_path or a "dir path" as .../remote_path/ using either the _rm or the _rm_file API, the folder is not deleted, only its content is.
Is there a workaround to force the deletion of the folder itself too? Should I use aiobotocore directly?

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants