FrankChen021 commented on a change in pull request #11078:
URL: https://github.com/apache/druid/pull/11078#discussion_r610285792
##########
File path:
server/src/main/java/org/apache/druid/server/audit/SQLAuditManagerConfig.java
##########
@@ -31,6 +31,9 @@
@JsonProperty
private boolean includePayloadAsDimensionInMetric = false;
+ @JsonProperty
+ private long maxPayloadSizeBytes = -1;
Review comment:
> Does HumanReadableBytes not work with -1?
`HumanReadableBytes` does not work with negative value from config since I
think there's no such scenario that a size could be negative. But it can accept
negative value from code.
Here I think there're some ways:
1. if -1 is allowed in configuration, we could remove that constraint in
`HumanReadableBytes`, and apply `HumanReadableBytesRange` annotation to make
sure its value is in valid range
2. or, if we leave that constraint, user must delete this configuration to
disable the check
3. or, provide an independent flag to enable or disable the check as
suggested by @jihoonson
IMO, 3rd is better.
> I noticed we have no unit tests for HumanReadableBytes. It would be nice
to have some 🙂.
`HumanReadableBytes` has been covered by UT in
[HumanReadableBytesTest](https://github.com/apache/druid/blob/master/core/src/test/java/org/apache/druid/java/util/common/HumanReadableBytesTest.java)
file.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
