Closed
Conversation
|
go-test-coverage report: |
george-zubrienko
approved these changes
Dec 19, 2025
| "testing" | ||
| ) | ||
|
|
||
| func Test_StreamAdded(t *testing.T) { |
Contributor
There was a problem hiding this comment.
maybe try to create a test-only plugin CRD and utilize k8s testing framework instead - if possible.
| s.logger.V(0).Info("Backfill requested for stream", "streamId", streamEvent.StreamId().String()) | ||
| err := s.jobManager.EnsureStopped(streamEvent.StreamId()) | ||
| if err != nil { | ||
| s.logger.Error(err, "Failed to stop job for reload request", "streamId", streamEvent.StreamId().String()) |
Contributor
There was a problem hiding this comment.
Suggested change
| s.logger.Error(err, "Failed to stop job for reload request", "streamId", streamEvent.StreamId().String()) | |
| s.logger.Error(err, "Failed to stop job for backfill request", "streamId", streamEvent.StreamId().String()) |
| s.logger.V(0).Info("Stream suspended", "streamId", streamEvent.StreamId().String()) | ||
| err := s.jobManager.EnsureStopped(streamEvent.StreamId()) | ||
| if err != nil { | ||
| s.logger.Error(err, "Failed to stop job for suspended stream", "streamId", streamEvent.StreamId().String()) |
Contributor
There was a problem hiding this comment.
i'd propose (not in this PR) to fire an event through event recorder that will display on the related CR - if possible
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #144
Scope
This pull request introduces a new event-driven stream handling architecture for the streaming subsystem, focusing on improving modularity, testability, and type safety. It adds several new interfaces and types for handling streaming events, jobs, and repositories, and refactors the controller factory and handle to use these abstractions. The changes also include improvements to queue management and event processing, as well as new helper utilities for testing.
Core architecture and event handling:
Handlerclass that handles stream events.JobManager,StreamManager,StreamRepository, andStreamDefinitionto abstract job and stream management,StreamEvent,EventType, andStreamIdentitytypes to standardize event handling and identity representation.Queue and controller refactoring:
Changed the controller factory and handle to use a strongly typed
workqueue.TypedRateLimitingInterface[StreamEvent]instead ofanyTesting and utilities:
Added a new helper in
tests/helpers.goto execute a command and load a Kubernetes config, supporting flexible test environments.Updated
generate.goto add amockgendirective for generating mocks for the new stream interfaces, improving testability.NOTE: tests for this code are not completed yet
Checklist
latestcommit.