translate Retry to `Base.retry` by ericphanson · Pull Request #548 · JuliaCloud/AWS.jl

ericphanson · 2022-05-11T14:24:00Z

I'm not sure we should merge this as-is, because before we could do immediate retries OR delay retries, and now we can only do delay retries. ~~I'm also not 100% sure the delay amounts are the same.~~

The functional approach is nicer than the macro approach though in my opinion, ~~since the retries don't need to be literals~~. So we could let folks request more retries on a per-request basis, or even put it in the Backend struct.

edit 1: Retry.jl expands to a for-loop, so non-literals are OK. I thought it unrolled the loop.

edit 2: the delay amounts are definitely not the same, but that's a good thing- now we copy the upstream recommendation from https://docs.aws.amazon.com/sdkref/latest/guide/feature-retry-behavior.html. In particular, putting a cap of 20s means we can increase the retries further without danger, xref #550 (we can't do that with Retry.jl since the retries are hard-coded with no cap and get too big- ref #543 (comment)).

12 May update:

I'm now in favor of merging this, because I want to get in #550. The first few retries are pretty short, e.g.

julia> collect(AWS.AWSExponentialBackoff(; max_attempts=10))
9-element Vector{Any}:
  0.3365311959885935
  1.2370247946870188
  2.925043257220695
 12.165245886682294
 20.0
 20.0
  5.5969335490336505
 20.0
 20.0

So I think having a short delay instead of an immediate retry in a few cases is probably OK. Also, the AWS docs don't talk about non-delayed retries, they seem to only do delayed retries.

ericphanson · 2022-05-11T14:45:50Z

Going to check on CI:

bors try

bors · 2022-05-11T14:48:59Z

try

Build failed:

Julia 1 - ubuntu-latest - x64

ericphanson · 2022-05-11T15:06:14Z

bors try

bors · 2022-05-11T15:11:23Z

try

Build failed:

Julia 1 - ubuntu-latest - x64

ericphanson · 2022-05-11T17:31:01Z

bors try

bors · 2022-05-11T17:35:17Z

try

Build failed:

Julia 1 - ubuntu-latest - x64

ericphanson · 2022-05-11T17:38:52Z

bors try

ericphanson · 2022-05-11T17:39:19Z

@@ -0,0 +1 @@
+style="blue"


This lets me use "format" command in VSCode

bors · 2022-05-11T17:43:17Z

try

Build failed:

Julia 1.5 - ubuntu-latest - x64

ericphanson · 2022-05-11T17:54:52Z

bors try

bors · 2022-05-11T18:00:04Z

try

Build succeeded:

ericphanson · 2022-05-12T17:43:42Z

bors try

bors · 2022-05-19T13:03:44Z

try

Build failed:

Julia 1.3 - ubuntu-latest - x64

ericphanson · 2022-05-19T13:10:08Z

bors try

bors · 2022-05-19T13:16:43Z

try

Build succeeded:

Co-authored-by: Curtis Vogt <curtis.vogt@gmail.com>

ericphanson · 2022-05-19T15:18:14Z

bors try

bors · 2022-05-19T15:21:08Z

try

Build failed:

Julia 1 - ubuntu-latest - x64

ericphanson · 2022-05-19T18:19:55Z

bors try

bors · 2022-05-19T18:23:53Z

try

Build failed:

Julia 1 - ubuntu-latest - x64

This reverts commit 6a96fae.

ericphanson · 2022-05-19T18:32:38Z

bors try

bors · 2022-05-19T18:38:44Z

try

Build succeeded:

ericphanson · 2022-05-19T18:44:21Z

Ok! I don't really want to do more formatting changes since it's kind of pain to get it through Bors again. Are there any other changes we should make here? Or is it good to go?

omus

LGTM

Co-authored-by: Curtis Vogt <curtis.vogt@gmail.com>

ericphanson · 2022-05-25T15:01:57Z

bors r+

bors · 2022-05-25T15:08:19Z

Build succeeded:

549: Add debug logging messages on retry r=mattBrzezinski a=ericphanson Closes #514 Based on #548 The logging messages are structured so the string is a human-readable summary of the message, and then the fields are more structured for later analysis, e.g. `retry::Bool`, `reason::String`, etc. Additionally, the id of the log message will be useful for aggregation. Co-authored-by: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Co-authored-by: Matt Brzezinski <matt.brzezinski@beacon.bio>

ericphanson added 2 commits May 11, 2022 16:20

translate Retry to Base.retry

5569caf

update one more retry

5b3f86b

bors Bot added a commit that referenced this pull request May 11, 2022

Try #548:

00f813b

ericphanson added 2 commits May 11, 2022 16:54

fix logic

3c129fa

actually call the function

00f24ec

bors Bot added a commit that referenced this pull request May 11, 2022

Try #548:

4e66053

try to fix things

f226445

bors Bot added a commit that referenced this pull request May 11, 2022

Try #548:

c145a2f

match number of retries properly

e91a4f1

bors Bot added a commit that referenced this pull request May 11, 2022

Try #548:

bbe67db

ericphanson commented May 11, 2022

View reviewed changes

Comment thread .JuliaFormatter.toml

@@ -0,0 +1 @@

style="blue"

ericphanson May 11, 2022

Copy link
Copy Markdown

Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lets me use "format" command in VSCode

fix

8616f1e

bors Bot added a commit that referenced this pull request May 11, 2022

Try #548:

bc3b2b4

ericphanson requested review from mattBrzezinski and omus May 11, 2022 18:02

ericphanson mentioned this pull request May 11, 2022

Add debug logging messages on retry #549

Closed

add and use AWSExponentialBackoff

cf9c55a

bors Bot added a commit that referenced this pull request May 12, 2022

Try #548:

7858097

ericphanson mentioned this pull request May 12, 2022

Increase some retries from 4 to 10 #550

Closed

don't use @compat; not working on 1.3

ee14b9f

bors Bot added a commit that referenced this pull request May 19, 2022

Try #548:

b14e515

ericphanson requested a review from omus May 19, 2022 13:18

omus reviewed May 19, 2022

View reviewed changes

Comment thread Project.toml Outdated

Comment thread Project.toml Outdated

Comment thread src/utilities/request.jl Outdated

Comment thread src/utilities/request.jl

Comment thread src/utilities/request.jl

Comment thread test/utilities.jl

Apply suggestions from code review

c841f52

Co-authored-by: Curtis Vogt <curtis.vogt@gmail.com>

bors Bot added a commit that referenced this pull request May 19, 2022

Try #548:

9984276

ericphanson added 2 commits May 19, 2022 20:19

restore function wrapper approach

64a6494

move upgrade_error out of submit_request

6a96fae

bors Bot added a commit that referenced this pull request May 19, 2022

Try #548:

9aab953

Revert "move upgrade_error out of submit_request"

f59aa6c

This reverts commit 6a96fae.

bors Bot added a commit that referenced this pull request May 19, 2022

Try #548:

6ac4e04

ericphanson requested a review from omus May 19, 2022 18:44

omus approved these changes May 19, 2022

View reviewed changes

Comment thread test/utilities.jl Outdated

Update test/utilities.jl

15c6f60

Co-authored-by: Curtis Vogt <curtis.vogt@gmail.com>

bors Bot merged commit d52c639 into master May 25, 2022

bors Bot deleted the eph/base-retry branch May 25, 2022 15:08

Uh oh!

Conversation

ericphanson commented May 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericphanson commented May 11, 2022

Uh oh!

bors Bot commented May 11, 2022

try

Uh oh!

ericphanson commented May 11, 2022

Uh oh!

bors Bot commented May 11, 2022

try

Uh oh!

ericphanson commented May 11, 2022

Uh oh!

bors Bot commented May 11, 2022

try

Uh oh!

ericphanson commented May 11, 2022

Uh oh!

ericphanson May 11, 2022

Choose a reason for hiding this comment

Uh oh!

bors Bot commented May 11, 2022

try

Uh oh!

ericphanson commented May 11, 2022

Uh oh!

bors Bot commented May 11, 2022

try

Uh oh!

ericphanson commented May 12, 2022

Uh oh!

bors Bot commented May 19, 2022

try

Uh oh!

ericphanson commented May 19, 2022

Uh oh!

bors Bot commented May 19, 2022

try

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ericphanson commented May 19, 2022

Uh oh!

bors Bot commented May 19, 2022

try

Uh oh!

ericphanson commented May 19, 2022

Uh oh!

bors Bot commented May 19, 2022

try

Uh oh!

ericphanson commented May 19, 2022

Uh oh!

bors Bot commented May 19, 2022

try

Uh oh!

ericphanson commented May 19, 2022

Uh oh!

omus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ericphanson commented May 25, 2022

Uh oh!

bors Bot commented May 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

ericphanson commented May 11, 2022 •

edited

Loading