Skip to content

feat: add builder for GenericRetry and deprecate mutable setters - #3648

Open
TQJADE wants to merge 1 commit into
operator-framework:mainfrom
TQJADE:retry
Open

TQJADE wants to merge 1 commit into
operator-framework:mainfrom
TQJADE:retry

Conversation

@TQJADE

@TQJADE TQJADE commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Adds a builder for GenericRetry as a non-breaking first step towards
making it immutable in v6. The mutating methods are deprecated for
removal, so users can migrate now and the setters can be dropped in the
next major version. More: #2935

Summary by CodeRabbit

  • New Features

    • Added a builder for configuring retry attempts, intervals, and retry strategies. Existing retry configurations can also be copied and adjusted.
  • Documentation

    • Updated retry examples to use the builder and explain how to customize configurations. Existing setter-based configuration methods are deprecated and are planned for removal in a future major version.

Copilot AI balanced review requested due to automatic review settings October 14, 2026 18:21
@openshift-ci
openshift-ci Bot requested review from metacosm and xstefank October 14, 2026 18:21
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e8317e7e-a24a-426b-ab10-33fa38501d2e
📥 Commits

Reviewing files that changed from the base of the PR and between b300073 and 53f7269.

📒 Files selected for processing: 13
  • docs/content/en/docs/documentation/error-handling-retries.md
  • docs/content/en/docs/documentation/testing.md
  • operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java
  • operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java
  • operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java
  • operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecutionTest.java
  • operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryTest.java
  • operator-framework/src/main/java/io/javaoperatorsdk/operator/config/loader/ConfigLoader.java
  • operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/errorstatushandler/ErrorStatusHandlerIT.java
  • operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/retry/RetryIT.java
  • operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/retry/RetryIntervalHonoredOnFrequentEventsIT.java
  • operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/retry/RetryMaxAttemptIT.java
  • operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/eventing/TriggerReconcilerOnAllEventIT.java

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

GenericRetry gains a builder API and toBuilder() support. Existing mutators are marked for removal. Configuration code, tests, and documentation examples now use the builder while preserving their configured retry values.

Changes: Retry Configuration

Layer / File(s) Summary
Builder API and behavior
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java, operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryTest.java
GenericRetry adds a builder with default settings, configuration methods, copying through toBuilder(), and build operations. Existing mutators are marked for removal. Tests cover builder values, copied instances, and factory methods.
Builder adoption and validation
operator-framework/src/main/java/io/javaoperatorsdk/operator/config/loader/ConfigLoader.java, operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/{event,retry}/*, operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/..., operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/..., docs/content/en/docs/documentation/{error-handling-retries.md,testing.md}
ConfigLoader and retry tests configure retries with the builder API. Documentation examples show builder usage and describe the deprecation of set* methods.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Suggested reviewers: csviri

Merge Risk: ⚪ Minimal · up to 53f72

The retry configuration retains its previous values, and no actionable merge-blocking issue remains after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 19.30% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 57 functions across 11 files. (2 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main changes: adding a builder for GenericRetry and deprecating mutable setters.
Full details: Docstring Coverage

Explanation

Docstring coverage is 19.30% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 57 functions across 11 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Static review found no blocking issues, with existing retry behavior preserved and focused builder coverage added; tests were not executed.

0 open findings

What changed in this PR

Refactors GenericRetry toward builder-based configuration while preserving existing retry behavior and annotation support.

Changes:

  • Adds a builder, toBuilder(), and diagnostic toString(); deprecates mutable configuration methods.
  • Migrates configuration loading, tests, and documentation to the builder API.
  • Adds tests for builder defaults, configuration copying, and independent instances.
File Description
operator-framework/​src/​test/​java/​io/​javaoperatorsdk/​operator/​baseapi/​triggerallevent/​eventing/​TriggerReconcilerOnAllEventIT.java Migrates event-trigger retry setup.
operator-framework/​src/​test/​java/​io/​javaoperatorsdk/​operator/​baseapi/​retry/​RetryMaxAttemptIT.java Migrates retry-limit setup.
operator-framework/​src/​test/​java/​io/​javaoperatorsdk/​operator/​baseapi/​retry/​RetryIT.java Migrates retry setup.
operator-framework/​src/​test/​java/​io/​javaoperatorsdk/​operator/​baseapi/​retry/​RetryIntervalHonoredOnFrequentEventsIT.java Migrates retry-interval setup.
operator-framework/​src/​test/​java/​io/​javaoperatorsdk/​operator/​baseapi/​errorstatushandler/​ErrorStatusHandlerIT.java Migrates error-handler retry setup.
operator-framework/​src/​main/​java/​io/​javaoperatorsdk/​operator/​config/​loader/​ConfigLoader.java Builds retries from loaded configuration.
operator-framework-core/​src/​test/​java/​io/​javaoperatorsdk/​operator/​processing/​retry/​GenericRetryTest.java Adds builder and factory coverage.
operator-framework-core/​src/​test/​java/​io/​javaoperatorsdk/​operator/​processing/​retry/​GenericRetryExecutionTest.java Migrates retry-execution fixtures.
operator-framework-core/​src/​test/​java/​io/​javaoperatorsdk/​operator/​processing/​event/​ReconciliationDispatcherTest.java Migrates zero-retry configuration.
operator-framework-core/​src/​test/​java/​io/​javaoperatorsdk/​operator/​processing/​event/​EventProcessorTest.java Migrates event-processing retry fixtures.
operator-framework-core/​src/​main/​java/​io/​javaoperatorsdk/​operator/​processing/​retry/​GenericRetry.java Introduces builder API and deprecates mutators.
docs/​content/​en/​docs/​documentation/​testing.md Updates the testing example.
docs/​content/​en/​docs/​documentation/​error-handling-retries.md Documents builder usage and deprecation plans.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@TQJADE TQJADE changed the title GenericRetry refactor feat: add builder for GenericRetry and deprecate mutable setters Oct 14, 2026

This branch has not been deployed

No deployments
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