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

Extend Retry logic to address OSErrors. #493

Closed
wants to merge 6 commits into from
Closed

Conversation

alxmrs
Copy link
Contributor

@alxmrs alxmrs commented Aug 29, 2022

I've encountered a case where large bulk deletions throws an OSError. The GCS documentation points out that this is a general problem (https://cloud.google.com/storage/docs/deleting-objects#delete-objects-in-bulk). I did not expect, however, that this would throw an OSError instead of a 500 error.

In any case, this PR extends gcsfs's retry logic to include internal errors that throw OSErrors. My hope is that this fixes an associated issue in Pangeo Forge Recipes (pangeo-forge/pangeo-forge-recipes#406).

alxmrs added 2 commits August 29, 2022 13:36
I've encountered a case where large bulk deletions throws an `OSError`. The GCS documentation points out that this is a general problem (https://cloud.google.com/storage/docs/deleting-objects#delete-objects-in-bulk). I did not expect, however, that this would throw an OSError instead of a 500 error.

In any case, this PR extends `gcsfs`'s retry logic to include internal errors that throw `OSError`s. My hope is that this fixes an associated issue in Pangeo Forge Recipes (pangeo-forge/pangeo-forge-recipes#406).
@martindurant
Copy link
Member

Is it possible to ascertain what specific OSError happens? A blanket retry on all OSErrors seems like a bad idea, since something like "no internet" should not be retriable.

@alxmrs
Copy link
Contributor Author

alxmrs commented Aug 31, 2022

It looks like my slow tests is, in fact, too slow. I'm happy to always skip this test.

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

Successfully merging this pull request may close these issues.

2 participants