Skip to content

Conversation

@tomleb
Copy link
Contributor

@tomleb tomleb commented Aug 28, 2025

POC

@juice928
Copy link

👋 Hi, I'm an automated AI code review bot. I ran some checks on this PR and found 2 points that might be worth attention (could be false positives, please use your judgment):

  1. Enhancing context propagation in the main controller loop

    • pkg/controller/controller.go:L202: pkg/controller/controller.go:L202
    • Impact: Using context.TODO() here might prevent in-flight handlers from receiving cancellation signals, potentially leading to resource hangs.
    • Suggestion: Consider replacing context.TODO() with the controller’s lifecycle context (e.g., c.ctx) to maintain a consistent cancellation chain.
  2. Utilizing the passed context in the dynamic controller's Update method

    • pkg/dynamic/controller.go:L326: pkg/dynamic/controller.go:L326
    • Impact: Ignoring the context argument means caller-defined deadlines and cancellations won't be respected by the underlying calls.
    • Suggestion: It would be great to name the context parameter and pass it through to the client calls to fully support the new interface.

If you find these suggestions disruptive, you can reply "stop" , and I'll automatically skip this repository in the future.

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