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

Request retries respect response #1057

Merged
merged 1 commit into from
May 26, 2023
Merged

Conversation

Drvi
Copy link
Collaborator

@Drvi Drvi commented May 26, 2023

I think we recently increased our usage of CapturedExceptions which broke isrecoverable, which dispatches on exception type and tries to recover exceptions by default.

Without this PR, the following would hang for 15 minutes.

import HTTP
HTTP.retryable(status::Int16) = false

using CloudStore
url = "azure://account/container/file.csv"
ok, host, account, container, blob = CloudStore.parseAzureAccountContainerBlob(url)
store =  CloudStore.Azure.Container(container, account; host)
retry_delays = ExponentialBackOff(n=12, max_delay=180, factor=3.0) # ~ 15 minutes of total delay time
credentials = nothing
CloudStore.get(store, blob; credentials, retry_delays, retries=length(retry_delays), verbose=2) # retries 403

This would also explain https://github.com/RelationalAI/raicode/pull/13231#discussion_r1180219818

@Drvi Drvi requested a review from quinnj May 26, 2023 10:18
@Drvi Drvi force-pushed the td-request-retry-respects-response branch 2 times, most recently from 9b75548 to f97be79 Compare May 26, 2023 10:56
@Drvi Drvi force-pushed the td-request-retry-respects-response branch from f97be79 to c0a5240 Compare May 26, 2023 11:07
@Drvi
Copy link
Collaborator Author

Drvi commented May 26, 2023

Not sure whats up with CI, but I'm getting failures even locally on master

@quinnj
Copy link
Member

quinnj commented May 26, 2023

Yeah, CI is broken for unrelated reasons (a test server we were using for download testing change behavior)

@quinnj quinnj merged commit 8e290b9 into master May 26, 2023
@quinnj quinnj deleted the td-request-retry-respects-response branch May 26, 2023 15:56
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