jtuglu1 commented on code in PR #18480:
URL: https://github.com/apache/druid/pull/18480#discussion_r2328457981
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/autoscaler/LagBasedAutoScalerConfig.java:
##########
@@ -212,6 +218,15 @@ public AggregateFunction getLagAggregate()
return lagAggregate;
}
+ @Override
+ @JsonProperty
+ @Nullable
+ @JsonInclude(JsonInclude.Include.NON_NULL)
Review Comment:
> I don't quite follow. A non-nullable field would never be null anyway.
Of course. I think I was just trying to point out that we should have a
standard for leaving null (but optional) fields in a config, versus keeping
them out.
> For anyone reading the json, it doesn't make a real difference if the
field is absent or if it is assigned "field": null.
I think it does. I believe what Gian was implying was that for non-required
fields, having null values in the config looks weird/adds bloat.
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/autoscaler/LagBasedAutoScalerConfig.java:
##########
@@ -212,6 +218,15 @@ public AggregateFunction getLagAggregate()
return lagAggregate;
}
+ @Override
+ @JsonProperty
+ @Nullable
+ @JsonInclude(JsonInclude.Include.NON_NULL)
Review Comment:
> I don't quite follow. A non-nullable field would never be null anyway.
Of course. I think I was just trying to point out that we should have a
standard for leaving null (but optional) fields in a config, versus keeping
them out.
> For anyone reading the json, it doesn't make a real difference if the
field is absent or if it is assigned "field": null.
I think it does. I believe what Gian was implying was that for non-required
fields, having null values in the config looks weird/adds bloat.
--
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]
