Skip to content

feat: change operations retry logic to be opt-in #39

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jkongie
Copy link
Collaborator

@jkongie jkongie commented Apr 29, 2025

BREAKING CHANGE: The retry logic for operations is now opt-in. To enable the retry, the caller must now provide the WithRetry option to the ExecuteOperation. This change is intended to provide greater control over the retry behavior and to avoid unintended retries in certain scenarios.

@jkongie jkongie requested a review from a team as a code owner April 29, 2025 05:33
Copy link

changeset-bot bot commented Apr 29, 2025

🦋 Changeset detected

Latest commit: 9dec4fc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
chainlink-deployments-framework Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ajaskolski ajaskolski requested a review from Copilot April 29, 2025 15:29
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request makes the operations retry logic opt-in, allowing callers to enable retry behavior by providing the WithRetry option. Key changes include:

  • Removing explicit generic type parameters from NewReport calls in tests and report generation.
  • Refactoring the retry configuration to use an Enabled flag rather than a DisableRetry flag.
  • Updating tests to reflect the new retry logic and naming conventions.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
operations/report_test.go Updated NewReport invocation to match the new generic signature.
operations/report.go Adjusted genericReport usage to remove explicit type arguments.
operations/execute_test.go Modified test cases names and options to align with opt-in retry logic.
operations/execute.go Refactored retry configuration and logic; now using an Enabled flag.
.changeset/tangy-plums-draw.md Updated changeset to document the breaking change.

@@ -18,16 +18,59 @@ type ExecuteConfig[IN, DEP any] struct {
type ExecuteOption[IN, DEP any] func(*ExecuteConfig[IN, DEP])

type RetryConfig[IN, DEP any] struct {
// DisableRetry disables the retry mechanism if set to true.
DisableRetry bool
// Enabled dermines if the retry is enabled for the operation.
Copy link
Preview

Copilot AI Apr 29, 2025

Choose a reason for hiding this comment

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

There's a spelling mistake in the comment; change 'dermines' to 'determines'.

Suggested change
// Enabled dermines if the retry is enabled for the operation.
// Enabled determines if the retry is enabled for the operation.

Copilot uses AI. Check for mistakes.

BREAKING CHANGE: The retry logic for operations is now opt-in. To
enable the retry, the caller must now provide the `WithRetry` option
to the `ExecuteOperation`. This change is intended to provide greater
control over the retry behavior and to avoid unintended retries in
certain scenarios.
@jkongie jkongie force-pushed the CLD-201/opt-in-op-retry branch from 87594f7 to 9dec4fc Compare April 29, 2025 15:32
@jkongie jkongie marked this pull request as draft April 30, 2025 06:18
@jkongie jkongie marked this pull request as ready for review May 1, 2025 05:53
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