Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

feat: handle retrying on the client, not service #328

Merged
merged 4 commits into from
Aug 22, 2022
Merged

Conversation

noahnu
Copy link
Contributor

@noahnu noahnu commented Aug 18, 2022

The lambda has a time limit so we should handle retrying on the client, not the service.

TODO:

  • Remove async-retry. Use a for...loop

QAd with local invoke.

@noahnu noahnu requested a review from eastenluis August 18, 2022 20:50
packages/client/src/runTests.ts Outdated Show resolved Hide resolved
const invokeResult = await invokeBackend.invoke({
config: {
...config,
retryCount: 0, // client will take care of retrying
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does the invoke need the retryCount? I don't see how the thing being retried gets to interact with the count.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The retry count gets passed to the service currently because the service used to do the retrying. This zeroes it out so only the client retries.

@tophat-opensource-bot
Copy link
Collaborator

tophat-opensource-bot commented Aug 18, 2022

Sanity Runner Publish Preview

This Pull Request introduces the following changes:

Changelog

6.5.0 "sanity-runner-client" (2022-08-22)

Features

  • handle retrying on the client, not service (8b90e92)

@noahnu noahnu requested a review from shawndrape August 19, 2022 12:30
@noahnu noahnu marked this pull request as draft August 19, 2022 20:28
@noahnu noahnu force-pushed the retry_client_side branch from 0e49a47 to aa0005c Compare August 19, 2022 20:35
@noahnu noahnu marked this pull request as ready for review August 22, 2022 13:34
@noahnu noahnu merged commit ae3f699 into master Aug 22, 2022
@noahnu noahnu deleted the retry_client_side branch August 22, 2022 13:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants