Skip to content

Conversation

@Donnerbart
Copy link
Contributor

We're digging into a reported operator health issue right now, and I noticed the for for typo in the debug log.

I also noticed a RetryAndRescheduleTimerEventSource with status UNKNOWN in the event source health indicators, caused by TimerEventSource not implementing the getStatus() method. If that was intentional (since we don't know if the actual timer tasks have been executed successfully), I can drop that commit again.

Copilot AI review requested due to automatic review settings October 24, 2025 14:38
@openshift-ci openshift-ci bot requested review from csviri and xstefank October 24, 2025 14:38
Copy link

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 PR fixes a typo in a debug log message and implements the getStatus() method for TimerEventSource to properly report health status instead of defaulting to UNKNOWN.

Key Changes:

  • Corrected "for for" typo in InformerWrapper debug log
  • Implemented getStatus() method in TimerEventSource to return HEALTHY/UNHEALTHY based on running state
  • Refactored InformerWrappingEventSourceHealthIndicator to use more concise stream operation

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
TimerEventSource.java Adds getStatus() method implementation to report HEALTHY when running, UNHEALTHY when stopped
TimerEventSourceTest.java Adds assertions to verify status is HEALTHY/UNHEALTHY in appropriate test scenarios
InformerWrapper.java Fixes typo "for for" → "for" and adjusts spacing in debug log message
InformerWrappingEventSourceHealthIndicator.java Simplifies stream logic using anyMatch instead of filter/findAny
PollingEventSourceTest.java Removes unnecessary throws declaration from test method

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@csviri
Copy link
Collaborator

csviri commented Oct 24, 2025

Hi @Donnerbart I think this is reasonable to change it this way. Thank you!

Copy link
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

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

LGTM

@csviri
Copy link
Collaborator

csviri commented Oct 24, 2025

I did not checked yet, but this might be also interesting:
#2978

@csviri csviri requested a review from metacosm October 24, 2025 15:15
Signed-off-by: David Sondermann <david.sondermann@hivemq.com>
Signed-off-by: David Sondermann <david.sondermann@hivemq.com>
Signed-off-by: David Sondermann <david.sondermann@hivemq.com>
Signed-off-by: David Sondermann <david.sondermann@hivemq.com>
@csviri csviri force-pushed the improvement/fix-typo-informer-wrapper branch from 4247e2a to 4eec857 Compare October 27, 2025 08:25
@csviri csviri merged commit 6127066 into operator-framework:main Oct 27, 2025
25 checks passed
@csviri
Copy link
Collaborator

csviri commented Oct 27, 2025

thank you @Donnerbart !

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