-
Notifications
You must be signed in to change notification settings - Fork 243
next (v5.6; do not merge) #3474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 16 commits
fbce8b2
7e5a900
3fecf20
873eca1
0ddecbd
eb7958d
fffe39c
9e5927a
8ad1115
14912ba
510e173
907fe61
9272c99
aa809d4
db9a0fa
592bad3
d4bd3da
b5022e2
484727d
51490a2
9010df1
eba57e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -36,13 +36,16 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.fabric8.kubernetes.client.utils.KubernetesSerialization; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.api.monitoring.Metrics; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.api.reconciler.Context; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.api.reconciler.Experimental; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.api.reconciler.Reconciler; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowFactory; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerEventSource; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.processing.event.source.informer.pool.DefaultInformerPool; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerPool; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** An interface from which to retrieve configuration information. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public interface ConfigurationService { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -476,4 +479,27 @@ default boolean useSSAToPatchPrimaryResource() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default boolean cloneSecondaryResourcesWhenGettingFromCache() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * The informer pool used to create and (when using the default, sharing pool) share the informers | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * backing the event sources of all controllers managed by this {@code ConfigurationService}. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * <p><strong>Implementations must return the same instance on every call.</strong> The pool is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * effectively a per-{@code ConfigurationService} singleton: controllers share informers only if | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * they resolve the same pool, and reference counting / informer shutdown are only correct if | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * {@code getInformer} and {@code releaseInformer} operate on that same instance. This is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * intentionally not a {@code default} method, since a {@code default} could not cache the result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Wrap the Javadoc line to stay within 100 characters. Line 491 exceeds the Java line-length limit. Move the As per coding guidelines: “Limit Java source lines to 100 characters.” 🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * and would hand out a fresh (unshared) pool on each call; {@link AbstractConfigurationService} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * provides a cached implementation backed by the default sharing pool. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return the informer pool for this configuration service | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Experimental( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Only the configuration API around informer pooling could still change in a" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| + " non-backwards-compatible way, the pooling itself is prod ready.") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default InformerPool informerPool() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var pool = new DefaultInformerPool(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pool.setConfigurationService(this); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return pool; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+483
to
+503
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Make Line 487 requires the same pool instance on every call. Lines 500-503 create a new pool on every call. A custom Keep the cached implementation in Proposed fix- default InformerPool informerPool() {
- var pool = new DefaultInformerPool();
- pool.setConfigurationService(this);
- return pool;
- }
+ InformerPool informerPool();📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
csviri marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the grammar in the project descriptions.
is an.While ... butconstruction.not yet released as open source.cloud-native.Proposed wording
Also applies to: 75-76, 80-81
🤖 Prompt for AI Agents
Source: Linters/SAST tools