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]