Skip to content

WIP: Retries #232

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

WIP: Retries #232

wants to merge 6 commits into from

Conversation

gjoseph92
Copy link
Owner

Support retrying errors matching a given pattern.

This also moves error handling logic (both retires and nodata) up to the to_dask level, and out of Readers.

Closes #18.

Still need to do sleep/backoff, etc. Hoping to remove NodataReader too.
When an `open` fails and we'd return nodata instead, we don't cache that anymore. So if the asset doesn't exist, we'll try to open it for every chunk. Unclear how much of a performance impact this will have. It's probably not ideal, because combining it with retries could be nice: retry errors first, then give up and use nodata if they persist?
got lost in #221, which was a reasonable change, but turns out there was still a test importing it
TODO:
- sleep & backoff
- figure out how to handle sleep in tests
@charalamm
Copy link

charalamm commented Dec 8, 2023

Hi @gjoseph92 thanks for this feature. I had a look an although I have not tested the code it looks good. The way of using this feature seems versatile so imho, nice, I would defiantly use this feature!

I see that you have not included the CPL_VSIL_CURL_NON_CACHED gdal parameter. Does the retry work without it or the users need to add in gdal_env?

Also, I would think that the possibility to add a delay between retries would be also useful.

@charalamm
Copy link

charalamm commented Jan 19, 2024

Hello @gjoseph92 . Do you think you will merge this anytime soon?
Let me know if I can help somehow

gjoseph92 added a commit that referenced this pull request Aug 10, 2024
Opted not to refactor `NodataReader` as described in comments because #232 removes it all together, and I don't need even more merge conflicts someday.

Fixes #243
gjoseph92 added a commit that referenced this pull request Aug 10, 2024
Opted not to refactor `NodataReader` as described in comments because #232 removes it all together, and I don't need even more merge conflicts someday.

Fixes #243
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handling intermittent data retrieval errors (retries)
2 participants