Skip to content

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants