FrankChen021 commented on code in PR #19014:
URL: https://github.com/apache/druid/pull/19014#discussion_r3934180542


##########
extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/KubernetesTaskExecutionConfigResource.java:
##########
@@ -57,16 +57,18 @@ public class KubernetesTaskExecutionConfigResource
   private static final Logger log = new 
Logger(KubernetesTaskExecutionConfigResource.class);
   private final JacksonConfigManager configManager;
   private final AuditManager auditManager;
-  private AtomicReference<KubernetesTaskRunnerDynamicConfig> dynamicConfigRef 
= null;
+  private final KubernetesTaskRunnerEffectiveConfig effectiveConfig;
 
   @Inject
   public KubernetesTaskExecutionConfigResource(
       final JacksonConfigManager configManager,
-      final AuditManager auditManager
+      final AuditManager auditManager,
+      final KubernetesTaskRunnerEffectiveConfig effectiveConfig

Review Comment:
   [P2] Avoid requiring single-cluster config for multik8s
   
   Injecting `KubernetesTaskRunnerEffectiveConfig` makes this resource resolve 
the module's `KubernetesTaskRunnerStaticConfig`, whose `namespace` field is 
`@NotNull`. The `multik8s` path instead reads 
`MultipleKubernetesTaskRunnerConfig` and explicitly allows top-level 
`druid.indexer.runner.namespace` to be absent when cluster entries provide 
`taskNamespace` (for example with the custom-template adapter). On that 
supported configuration, provisioning this resource fails validation before 
GET/POST can run. Keep the resource dependency runner-agnostic or provide the 
active multik8s configuration.



##########
extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/DefaultKubernetesTaskRunnerDynamicConfig.java:
##########
@@ -35,14 +35,17 @@ public class DefaultKubernetesTaskRunnerDynamicConfig 
implements KubernetesTaskR
 
   @JsonCreator
   public DefaultKubernetesTaskRunnerDynamicConfig(
+      @Nullable
       @JsonProperty("podTemplateSelectStrategy") PodTemplateSelectStrategy 
podTemplateSelectStrategy,
+      @Nullable
       @JsonProperty("capacity") Integer capacity
   )
   {
     this.podTemplateSelectStrategy = podTemplateSelectStrategy;
     this.capacity = capacity;
   }
 
+  @Nullable

Review Comment:
   [P1] Duplicate @Nullable annotations prevent compilation
   
   The merge result places one `@Nullable` before `@Override` and another after 
`@JsonProperty` on both getters. `javax.annotation.Nullable` is not a 
repeatable annotation, so javac rejects this class with `Nullable is not a 
repeatable annotation type` before the module can build or test. Keep only one 
annotation on each getter (the second duplicate is at line 57).



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to