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

Recursion error on "fake" s3 directories #198

Closed
pjbull opened this issue Feb 2, 2022 · 3 comments · Fixed by #199
Closed

Recursion error on "fake" s3 directories #198

pjbull opened this issue Feb 2, 2022 · 3 comments · Fixed by #199
Labels
bug Something isn't working S3

Comments

@pjbull
Copy link
Member

pjbull commented Feb 2, 2022

This was brought up by @remi-braun in #148.

Sometimes, listing a directory with the S3Client results in a recursion error. This was seen on Ceph where "faked" S3 directories are more common.

@GeorgeSabu found that this is likely the same root cause as what was fixed in #190. We assume that all the "content" items from the API are files, but they can be directories as well. The way to detect which is a directory is to see if the size is 0, as is implemented here:
ElucidataInc@9c11c6a

@GeorgeSabu
Copy link
Contributor

GeorgeSabu commented Feb 3, 2022

@pjbull I am interested in contributing, here is the PR.

@jayqi
Copy link
Member

jayqi commented Feb 6, 2022

@pjbull per my comment here, I think that the root cause for this issue may be misidentified. I think it might actually be the issue with download_to + iterdir that was identified in #204, and in that case it would be fixed by #202.

@pjbull
Copy link
Member Author

pjbull commented Dec 18, 2022

Between #202 and #302, I think this should be fixed now. Closing unless we see this again.

@pjbull pjbull closed this as completed Dec 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working S3
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants