Skip to content

Conversation

@nkomonen-amazon
Copy link
Contributor

Problem:

We had 2 different implementation of similar code, between withRetries and waitUntil.

We couldn't easily merge them though since behavior was slightly different when it came to expected return types, and since one threw and the other did not.

  • waitUntil returns undefined if it never properly resolves
  • withRetries throws if it never properly resolves

Solution:

As a solution we keep both methods but they share the same underlying code.

  • waitUntil now has a backoff field to increase the delta between intervals
    • withRetries also has this
  • withRetries will retry even on a thrown exeception
    • on timeout it will throw the last exception it encountered

  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@nkomonen-amazon nkomonen-amazon requested a review from a team as a code owner January 17, 2025 00:10
@justinmk3
Copy link
Contributor

withRetries throws if it never properly resolves

can this behavior be controlled by a flag on waitUntil, so we don't need both functions?

@nkomonen-amazon nkomonen-amazon force-pushed the waitUntilBackoff branch 3 times, most recently from 84d9a91 to 978ed1a Compare January 17, 2025 21:15
Problem:

We had 2 different implementation of similar code, between
withRetries and waitUntil.

We couldn't easily merge them though since behavior was slightly different
when it came to expected return types, and since one threw and the other
did not.

- waitUntil returns undefined if it never properly resolves
- withRetries throws if it never properly resolves

Solution:

As a solution we keep both methods but they share the same underlying
code.

- waitUntil now has a backoff field to increase the delta between intervals
  - withRetries also has this
- withRetries will retry even on a thrown exeception
  - on timeout it will throw the last exception it encountered

Signed-off-by: nkomonen-amazon <[email protected]>
Signed-off-by: nkomonen-amazon <[email protected]>
@nkomonen-amazon
Copy link
Contributor Author

nkomonen-amazon commented Jan 21, 2025

/retryBuilds

Signed-off-by: nkomonen-amazon <[email protected]>
@nkomonen-amazon
Copy link
Contributor Author

/runIntegrationTests

Signed-off-by: nkomonen-amazon <[email protected]>
@justinmk3 justinmk3 merged commit cf700b6 into aws:master Jan 23, 2025
25 of 26 checks passed
kevluu-aws pushed a commit to kevluu-aws/aws-toolkit-vscode that referenced this pull request Jan 23, 2025
## Problem:
`withRetries` and `waitUntil` are two different interfaces for very
similar concepts.

## Solution:
- Add `backoff` and `retryOnFail` flags to `waitUntil()`.
    - `waitUntil(…, {retryOnFail:true})` returns `undefined` if it never
      properly resolves.
    - `waitUntil(…, {retryOnFail:false})` throws if it never properly
      resolves.
- Delete `withRetries()`.
chungjac pushed a commit to chungjac/aws-toolkit-vscode that referenced this pull request Jan 24, 2025
## Problem:
`withRetries` and `waitUntil` are two different interfaces for very
similar concepts.

## Solution:
- Add `backoff` and `retryOnFail` flags to `waitUntil()`.
    - `waitUntil(…, {retryOnFail:true})` returns `undefined` if it never
      properly resolves.
    - `waitUntil(…, {retryOnFail:false})` throws if it never properly
      resolves.
- Delete `withRetries()`.
s7ab059789 pushed a commit to s7ab059789/aws-toolkit-vscode that referenced this pull request Feb 19, 2025
## Problem:
`withRetries` and `waitUntil` are two different interfaces for very
similar concepts.

## Solution:
- Add `backoff` and `retryOnFail` flags to `waitUntil()`.
    - `waitUntil(…, {retryOnFail:true})` returns `undefined` if it never
      properly resolves.
    - `waitUntil(…, {retryOnFail:false})` throws if it never properly
      resolves.
- Delete `withRetries()`.
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.

3 participants