Dennis-Mircea commented on PR #1120: URL: https://github.com/apache/flink-kubernetes-operator/pull/1120#issuecomment-4614176127
> Overall I don't really understand why we are moving the event record from the factory field to the getResource. > > The eventRecorder is fixed and should be created once so there is not really a point in passing it always to getResourceContext, this is exactly the point of having the factory to avoid unnecessary object passing again and again. > > If we want to ensure that we use a single event recorder then lets use the same in the controller that we pass to the context factory Fair point on the threading. My intent was end-to-end ownership so each controller and the `FlinkService` it drives share the same recorder, and I leaned per-controller because that was already the more common shape as part of the existing implementation: session-job and snapshot controllers each create their own `EventRecorder` (only deployment reuses the factory's, and blue-green has none), and the `StatusRecorder` is per-controller too, so pairing a per-controller `EventRecorder` alongside it felt consistent. That said, I agree with your approach. I'll keep the single `EventRecorder` on the factory and pass that same instance to every controller, dropping the `getResourceContext` / `FlinkResourceContext` plumbing. That gives one consistent recorder everywhere without passing it on every call, and cleans up the session-job and snapshot controllers that were creating their own. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
