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.

Sign up for GitHub

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

Jump to bottom

feat(terraformlayer): exponential time before retry #102

Merged
merged 3 commits into from
May 9, 2023
Merged

feat(terraformlayer): exponential time before retry #102

merged 3 commits into from
May 9, 2023

Conversation

Thibaut-Padok
Copy link
Contributor

No description provided.

@Thibaut-Padok Thibaut-Padok added the enhancement New feature or request label Apr 18, 2023
@Thibaut-Padok Thibaut-Padok requested a review from spoukke April 18, 2023 12:32
@spoukke spoukke linked an issue Apr 18, 2023 that may be closed by this pull request
Implement exponential back-off #87
Closed
spoukke
spoukke suggested changes Apr 18, 2023
View reviewed changes
Copy link
Contributor

@spoukke spoukke left a comment

Choose a reason for hiding this comment

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

This is a good start. But:

  • I feel like the logic should go inside the controller and not at the types level
  • You do not reset the the annotation in the runner when a workflow is ok

api/v1alpha1/terraformlayer_types.go Outdated Show resolved Hide resolved
@Alan-pad
Copy link
Contributor

I think the exponential back-off implementation is ok (Maybe not making it a method of the TerraformLayer object)

But this should only apply to the PlanNeeded / ApplyNeeded state. The idea is we don't want to launch a plan/apply runner on a static period if we have an error

@Thibaut-Padok Thibaut-Padok force-pushed the feat/87-exponential-backoff branch from 1c2fa79 to 192003f Compare April 25, 2023 11:43
@Thibaut-Padok Thibaut-Padok requested a review from spoukke April 25, 2023 11:52
spoukke
spoukke suggested changes May 2, 2023
View reviewed changes
docs/contents/usage/README.md Outdated Show resolved Hide resolved
internal/controllers/terraformlayer/conditions_test.go Outdated Show resolved Hide resolved
internal/controllers/terraformlayer/states.go Outdated Show resolved Hide resolved
internal/controllers/terraformlayer/conditions.go Outdated Show resolved Hide resolved
internal/controllers/terraformlayer/conditions.go Outdated Show resolved Hide resolved
@Thibaut-Padok Thibaut-Padok force-pushed the feat/87-exponential-backoff branch 2 times, most recently from aa8affc to 55551b6 Compare May 2, 2023 13:20
@spoukke spoukke force-pushed the feat/87-exponential-backoff branch 2 times, most recently from 1cb61f2 to fd74658 Compare May 9, 2023 12:47
@spoukke spoukke force-pushed the feat/87-exponential-backoff branch from fd74658 to 2c861db Compare May 9, 2023 12:50
@spoukke spoukke force-pushed the feat/87-exponential-backoff branch from abeb4bc to cbdf4c8 Compare May 9, 2023 14:14
@spoukke spoukke merged commit 6f7d0bf into main May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alan-pad Alan-pad Alan-pad approved these changes

@spoukke spoukke spoukke approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement exponential back-off
3 participants
@Thibaut-Padok @Alan-pad @spoukke